[Pkg-kde-extras] Bug#432007: closed by Mark Purcell <msp at debian.org> (Re: Bug#432007: CVE-2007-1799: vulnerability in torrent.cpp)

[Mark Purcell]

On Fri, 6 Jul 2007, Steffen Joeris wrote:
> Thanks for investigating as well.
> I still have some concerns and maybe I am reading it wrong.
> When I download current ktorrent from unstable and look into the
> file "libktorrent/torrent/torrent.cpp", I do not see the if condition.
> IMHO adding the condition " if (!sd.contains("/") && !sd.contains(".."))"
> would fix the security issue, but the line is just missing.
> Maybe they found another way of fixing it and used different code. Can you
> please confirm that?

Hi Steffen,

Yes I had a look at that and saw that your patch wasn't included in the 
source, but the upstream changelog certainly states:

Changes in 2.1.4
- Fixed problem with files with .. in their name

In fact the upstream patch to fix this was a bit different to yours from the 
CVE.

Mark

Index: libktorrent/torrent/torrent.cpp
===================================================================
--- libktorrent/torrent/torrent.cpp     
(.../trunk/extragear/network/ktorrent/libktorrent/torrent/torrent.cpp 
(revision 648419)
+++ libktorrent/torrent/torrent.cpp     
(.../branches/extragear/kde3/network/ktorrent/libktorrent/torrent/torrent.cpp)  
(working copy)
@@ -20,6 +20,7 @@
  ***************************************************************************/
 #include <qfile.h>
 #include <qdatastream.h>
+#include <qstringlist.h>
 #include <util/log.h>
 #include <util/functions.h>
 #include <util/error.h>
@@ -164,19 +165,17 @@
                                        throw Error(i18n("Corrupted 
torrent!"));

                                QString sd = v->data().toString(encoding);
-                               // check for weirdness like .. and / ,
-                               // we don't want to write outside the user 
specified directories
-                               if (!sd.contains("/") && !sd.contains(".."))
-                               {
-                                       path += sd;
-                                       if (j + 1 < ln->getNumChildren())
-                                               path += bt::DirSeparator();
-                               }
+                               path += sd;
+                               if (j + 1 < ln->getNumChildren())
+                                       path += bt::DirSeparator();
                        }

                        // we do not want empty dirs
                        if (path.endsWith(bt::DirSeparator()))
                                continue;
+
+                       if (!checkPathForDirectoryTraversal(path))
+                               throw Error(i18n("Corrupted torrent!"));

                        BValueNode* v = d->getValue("length");
                        if (!v)
@@ -441,4 +440,10 @@
                        i++;
                }
        }
+
+       bool Torrent::checkPathForDirectoryTraversal(const QString & p)
+       {
+               QStringList sl = QStringList::split(bt::DirSeparator(),p);
+               return !sl.contains("..");
+       }
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
Url : http://lists.alioth.debian.org/pipermail/pkg-kde-extras/attachments/20070706/2af91338/attachment.pgp