← Back to team overview

linuxdcpp-team team mailing list archive

[Bug 550300] Re: Possible file corruption

 

Looks like the attached patch (by Big Muscle) solves the problem so the corrupted file can't go into the finished downloads folder anymore. However,
1. the wrong source kept in the queue resulting infinite redownload of the corrupted file.
2. a successful exploitation of this vulnerability needs that the source should not provide the full tiger tree so it may worth to rethink what to do when the full tree isn't available (there's also a resume problem in this case, explained at https://bugs.launchpad.net/dcplusplus/+bug/288756).?field.comment=Looks like the attached patch (by Big Muscle) solves the problem so the corrupted file can't go into the finished downloads folder anymore. However,
1. the wrong source kept in the queue resulting infinite redownload of the corrupted file.
2. a successful exploitation of this vulnerability needs that the source should not provide the full tiger tree so it may worth to rethink what to do when the full tree isn't available (there's also a resume problem in this case, explained at https://bugs.launchpad.net/dcplusplus/+bug/288756).

** Patch added: "tthcorruption_bm.patch"
   http://launchpadlibrarian.net/42307173/tthcorruption_bm.patch

** This bug has been flagged as a security vulnerability

-- 
Possible file corruption
https://bugs.launchpad.net/bugs/550300
You received this bug notification because you are a member of
Dcplusplus-team, which is a direct subscriber.

Status in DC++: Confirmed

Bug description:
This bug happens due to following source code:

FilteredFile.h, line 141: flushed = true;

Flushed flag is set to true although file is not flushed at this time. It causes that flushing in DownloadManager::endData() fails, because it thinks that file has already been flushed. If some system error (crash, hang etc.) occurs, file content will be lost, because it hasn't been flushed to disk.

Another problem with same source code is there. When user's HashData is not accessible (e.g. no access to file, file is corrupted etc.) and client can't get tree from such user, it uses only simple kind of verification - by TTH root. It's ok, but because DownloadManager/FilteredFile thinks that the file has already been flushed, it doesn't verify TTH root on file finish at all. So even though downloaded file's TTH root is different from original TTH root (i.e. file is corrupted), the file is normally finished and DC++ doesn't say any error (but it should say "TTH inconsistency" or something).

Possible solution for both problem would be to remove that "flushed = true" line and reset download position when flush fails on download finish (DownloadManager.cpp, line 328), so such segment wouldn't be marked as downloaded.





References