skip a torrent file that failed to parse - #2520
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A follow-up to #2519, and a correction of what I wrote there.
That change refuses unsafe names, and it does keep files inside the torrents folder - checked with crafted torrent files carrying "..", "/etc", a name of "../../evil" and a reserved Windows name: nothing was written outside. But my description claimed such a torrent "fails to parse entirely, rather than being partially accepted", and that part was not true. The parsing stops, yet the half built torrent was still added: the router created directories for those torrents and left a stray file.bin.part in the torrents folder.
Torrent::IsValid is added - a torrent needs a name, a piece length and either a length or a list of files - and ReadTorrentFile now skips anything that does not pass, with a line in the log.
Checked again with the same five files, four unsafe and one normal. Before: four "unsafe" lines, and the directories escape, absolute, good and normal appear along with file.bin.part. After: four "unsafe" lines each followed by "Invalid torrent file ... Skipped", only the directory of the legitimate torrent is created, and nothing else appears.
Builds without warnings, unit tests pass, C++17 syntax check is fine.