Bug 143637

Summary: The directory traversal fix is incomplete
Product: [Applications] ktorrent Reporter: Deathwing00 <deathwing00>
Component: generalAssignee: Joris Guisson <joris.guisson>
Severity: normal    
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Gentoo Packages   
OS: Linux   
Latest Commit: Version Fixed In:

Description Deathwing00 2007-03-30 19:26:40 UTC
Version:           2.1.2 (using KDE KDE 3.5.6)
Installed from:    Gentoo Packages
Compiler:          gcc-4.1.2 
OS:                Linux

From Matt Drew (aetius@gentoo.org):

"The directory traversal fix is incomplete. Cases such as '../' being inserted
into a bnode string in the path sequence will pass the filter they put in place
(which only checks if each string node is == "..").  I've verified this against
2.1.2 in the tree.

There's a pretty large number of cases that need to be checked - my suggestion
would be a "whitelist" of allowed characters in the strings that specify paths."
Comment 1 Deathwing00 2007-03-30 19:27:13 UTC
Downstream: https://bugs.gentoo.org/show_bug.cgi?id=170303
Comment 2 Joris Guisson 2007-03-31 12:03:56 UTC
You are right, but other then .. in a path, I don't think you can get outside of the user specified directories. We always prepend the user selected location before the path, so even if it starts with / it will not be a problem.
Comment 3 Joris Guisson 2007-03-31 12:07:03 UTC
SVN commit 648419 by guisson:

Fix directory traversal bug by making sure that path entries with .. and / in them are not allowed.

BUG: 143637

 M  +2 -2      torrent.cpp  

--- trunk/extragear/network/ktorrent/libktorrent/torrent/torrent.cpp #648418:648419
@@ -164,9 +164,9 @@
 					throw Error(i18n("Corrupted torrent!"));
 				QString sd = v->data().toString(encoding);
-				// check for weirdness like .. ,
+				// check for weirdness like .. and / ,
 				// we don't want to write outside the user specified directories
-				if (sd != "..") 
+				if (!sd.contains("/") && !sd.contains("..")) 
 					path += sd;
 					if (j + 1 < ln->getNumChildren())
Comment 4 Deathwing00 2007-03-31 13:11:47 UTC
Thank you. We'll add your patch downstream asap! :)
Comment 5 Joris Guisson 2007-03-31 18:57:46 UTC
There will be a new release tomorrow (we already had this planned), this will also contain the fix.
Comment 6 Deathwing00 2007-04-02 20:37:47 UTC
In our CVS. Thanks for the release!