Fix core subsystem logic and safety bugs#1723
Open
sirus20x6 wants to merge 1 commit intorakshasa:masterfrom
Open
Fix core subsystem logic and safety bugs#1723sirus20x6 wants to merge 1 commit intorakshasa:masterfrom
sirus20x6 wants to merge 1 commit intorakshasa:masterfrom
Conversation
- Fix inverted condition in apply_ip_tables_size_data (!=→==) - Fix always-true condition in remove_newlines (||→&&, add \r check) - Add NULL FILE* check in log_vmmap_dump before fprintf - Fix unreachable emit_changed() in ListFocus::erase by restructuring - Fix npos+7 wraparound in decode_data_uri by checking find() before offset - Fix off-by-one in signal handler bounds checks (>→>=) - Add input length validation in find_hex - Initialize group and check parse return in download_tracker_insert - Add mask_bits==0 special case to avoid UB shift by 32 - Catch all exceptions (not just bencode_error) for stack cleanup in parse_commands - Add NULL check before closedir in Directory::is_valid - Add empty file list check in retrieve_d_base_filename (matches retrieve_d_base_path)
7ead14e to
d1569d4
Compare
rakshasa
reviewed
Mar 14, 2026
| DownloadList::iterator | ||
| DownloadList::find_hex(const char* hash) { | ||
| if (strlen(hash) < 40) | ||
| return end(); |
Owner
There was a problem hiding this comment.
This should throw torrent::input_error when not 40.
| if (download->file_list()->is_multi_file()) | ||
| base = &download->file_list()->frozen_root_dir(); | ||
| else if (download->file_list()->empty()) | ||
| return std::string(); |
Owner
There was a problem hiding this comment.
Not sure this is right, as we don't want to return empty strings that get used in user's custom commands.
Empty should either throw an input or internal error (as empty filelist is supposed to be impossible to load iirc), or make a fake one using frozen_root_dir()+name.
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
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.
Summary
apply_ip_tables_size_datathat threw when table was found and dereferencedend()when notremove_newlines(||should be&&, add\rcheck)FILE*check inlog_vmmap_dumpbeforefprintfemit_changed()inListFocus::eraseby restructuring control flowstring::npos + 7wraparound indecode_data_uriby checkingfind()before adding offset>→>=for array indexed byHIGHEST_SIGNAL)find_hexto prevent out-of-bounds reads on short hash stringsgroupvariable and check parse return value indownload_tracker_insertmask_bits==0special case in CIDR parsing to avoid undefined behavior from shift by 32bencode_error) for stack cleanup incommand_function_call_objectclosedirinDirectory::is_validretrieve_d_base_filename(matches existing guard inretrieve_d_base_path)Test plan
ip_tables.size_datacommand with valid and invalid table names\r\nline endingslog.vmmap.dumpwith an invalid/unwritable path/0address range parsingfind_hexwith short/empty hash strings🤖 Generated with Claude Code