MAJOR BUG: Getting passkeys from other users...

[xirvik]

Hi,

One of our users just told us that he was able to download the metafile (the .torrent) of one of the downloads of a different user. Obviously this is a major bug in torrentflux-b4rt that everyone should patch as soon as possible so here's the details and the one line fix. Be aware that once the bug is known it's easy to exploit (no idea how to rush everyone to fix it other than releasing the details, though), so it should be taken seriously.

An user can get the metadata of a torrent if 1) He knows the name of the torrent 2) Obviously the torrent already exists in the server

Given a torrent with a name such as

Ubuntu.8.10.Server-CANONICAL.torrent

that already exists on the server, another user could upload another torrent with the name

ubuntu.8.10.server-canonical.torrent (not necessarily all lowercase - just one different character is enough) and get access to the first file.

The problem is that the file name comparison code is case sensitive, so it will not reject the second file (not that it should, anyway - but we all know that torrentflux-b4rt can't have two torrents with the same name), however the getOwner() function is not. You can see where this is going.

The GetOwner() function is defined in  inc/functions/functions.core.transfer.php

/**
 * get Owner
 *
 * @param $transfer
 * @return string
 */
function getOwner($transfer) {
        global $cfg, $db, $transfers;
        if (isset($transfers['owner'][$transfer])) {
                return $transfers['owner'][$transfer];
        } else {
                // Check log to see what user has a history with this file
                $transfers['owner'][$transfer] = $db->GetOne("SELECT user_id FROM tf_log WHERE file=".$db->qstr($transfer)." AND (action=".$db->qstr($cfg["constants"]["file_upload"])." OR action=".$db->qstr($cfg["constants"]["url_upload"])." OR action=".$db->qstr($cfg["constants"]["reset_owner"]).") ORDER BY time DESC");
                return ($transfers['owner'][$transfer] != "")
                        ? $transfers['owner'][$transfer]
                        : resetOwner($transfer); // try and get the owner from the stat file;
        }
}


The problem resides in the file='....' part, which is case insensitive. $db->GetOne will return one of the rows (so the first) returned by the query, assigning the wrong owner to the torrent. In MySQL, you can make the comparison case sensitive by changing that line to:

                $transfers['owner'][$transfer] = $db->GetOne("SELECT user_id FROM tf_log WHERE BINARY file=".$db->qstr($transfer)." AND (action=".$db->qstr($cfg["constants"]["file_upload"])." OR action=".$db->qstr($cfg["constants"]["url_upload"])." OR action=".$db->qstr($cfg["constants"]["reset_owner"]).") ORDER BY time DESC");


Xirvik

PS. Kudos to Dark21??? for let us know immediately.

[bb-tje]

What I always do is not let users see other users torrents.
That way this will also not happen.

[wonslung]

but the problem is that sometimes theres other ways to figure it out...
and it is a security problem FOR SURE

this same bug causes torrents to switch users sometimes and will sometimes make torrents download to the wrong dir...ti's quite annoying, i've known about it but never knew how to fix it...thanks


could you do me a favor though and put that stuff in code tags?
with code /code in brackets?

[xirvik]

What I always do is not let users see other users torrents.
That way this will also not happen.

We don't let users see others users torrent either. The problem here is that that torrentflux-b4rt is using the wrong owner for the torrent, therefore it's irrelevant whether the server is configured as public or private. The second user that uploads the torrent with the same (compared case-insensitively) name gets ownership of the existing torrent.

[steve51184]

so how do we apply this fix?

[Wwwildthing]

Take the system offline, open "inc\functions.core.transfer.php" and scroll down to line 889...

$transfers['owner'][$transfer] = $db->GetOne("SELECT user_id FROM tf_log WHERE file=".$db->qstr($transfer)." AND (action=".$db->qstr($cfg["constants"]["file_upload"])." OR action=".$db->qstr($cfg["constants"]["url_upload"])." OR action=".$db->qstr($cfg["constants"]["reset_owner"]).") ORDER BY time DESC");

replace it with...

$transfers['owner'][$transfer] = $db->GetOne("SELECT user_id FROM tf_log WHERE BINARY file=".$db->qstr($transfer)." AND (action=".$db->qstr($cfg["constants"]["file_upload"])." OR action=".$db->qstr($cfg["constants"]["url_upload"])." OR action=".$db->qstr($cfg["constants"]["reset_owner"]).") ORDER BY time DESC");

Save the file and restart.

[steve51184]

thank you very much for the reply but how do i take torrentflux offline and then when i've made the edit how do i restart it? :\

[bb-tje]

stop apache
start apache

if you are on a debian based system it's :

/etc/init.d/apache2 stop
/etc/init.d/apache2 start

[stupidul]

Thank you, xirvik for reporting this problem as well as the solution.

I'm sure the guys with access to the SVN will include this fix in the codebase, and maybe even release another beta?
Since this is a security risk (and not just a bug fix), a third beta should be released, to allow the normal users (those that don't poke their heads in the SVN) to move on. After all, it's been almost a year and a half since beta2.