CVE-2009-1760: libtorrent Arbitrary File Overwrite

This bug was discovered by Dimitris Glynos and I have to admit that I’m really sad for seeing Greek people moving to the other side like that. Anyway, libtorrent is a popular open source BitTorrent implementation. This issue affects libtorrent up to 0.14.3 release. Here is the buggy code from that release:

Copyright (c) 2003-2008, Arvid Norberg
All rights reserved.
       ...
        bool extract_single_file(lazy_entry const& dict, file_entry& target
                , std::string const& root_dir)
        {
       ...
               lazy_entry const* p = dict.dict_find("path.utf-8");
       ...
                for (int i = 0, end(p->list_size()); i < end; ++i)
                {
                        if (p->list_at(i)->type() != lazy_entry::string_t)
                                return false;
                        std::string path_element = p->list_at(i)->string_value();
                        if (path_element != "..")
                                target.path /= path_element;
                }
                verify_encoding(target);
                if (target.path.is_complete())
                        return false;
                return true;
        }

This function is used to extract a file from the .torrent file as it is implied by its name. What D. Glynos noticed is that the only check being performed at each entry (p->list_at(i)->string_value()) is just against “..” string. Because of this, an attacker is able access any file of the system using relative paths. D. Glynos gave an example in his advisory. To fix this they added a new function:

+       bool valid_path_element(std::string const& element)
+       {
+               if (element.empty()
+                       || element == "." || element == ".."
+                       || element[0] == '/' || element[0] == '\\'
+                       || element[element.size()-1] == ':')
+                       return false;
+               return true;
+       }
+
+       fs::path sanitize_path(fs::path const& p)
+       {
+               fs::path new_path;
+               for (fs::path::const_iterator i = p.begin(); i != p.end(); ++i)
+               {
+                       if (!valid_path_element(*i)) continue;
+                       new_path /= *i;
+               }
+               TORRENT_ASSERT(!new_path.is_complete());
+               return new_path;
+       }
+

And they updated the vulnerable routine to use it like this:

                        std::string path_element = p->list_at(i)->string_value();
-                       if (path_element != "..")
-                               target.path /= path_element;
+                       target.path /= path_element;
                }
+               target.path = sanitize_path(target.path);
                verify_encoding(target);
+               TORRENT_ASSERT(!target.path.is_complete());
+
                if (target.path.is_complete())

A similar patch was also performed in load_file() function of libtorrent. Once again, shame on us! I am completely disappointed of some Greek coders.