Bug 143637 - The directory traversal fix is incomplete
Summary: The directory traversal fix is incomplete
Status: RESOLVED FIXED
Alias: None
Product: ktorrent
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Platform: Gentoo Packages Linux
: NOR normal with 20 votes (vote)
Target Milestone: ---
Assignee: Joris Guisson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-03-30 19:26 UTC by Deathwing00
Modified: 2007-04-02 20:37 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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!