Skip to content

Remaining audit fixes pending validation and separation - #321

Closed
dajiaohuang wants to merge 8 commits into
rhash:masterfrom
dajiaohuang:fix-313-318-audit-findings
Closed

dajiaohuang wants to merge 8 commits into
rhash:masterfrom
dajiaohuang:fix-313-318-audit-findings

Conversation

@dajiaohuang

@dajiaohuang dajiaohuang commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

This draft covers issue-linked fixes for #313, #314, #316, #317 and #318. The independent torrent-path allocation leak (#315) is handled in #323 and removed from this diff.

#313 is reproduced through an actual configuration file: AddressSanitizer reports a stack-buffer-overflow in str_trim for an empty option name. The fix passes the saved regression and all 26 CLI groups on Linux/GCC 13.3 with SHA-NI disabled.

#314 is reproduced with a short filename using AddressSanitizer pointer-pair checks: the base reports an invalid pointer pair in find_embedded_crc32; the bounded scan and saved filename regressions pass.

#316 is reproduced by injecting one aligned-allocation failure into the checkout-built librhash: the base reports a UBSan null-member access in rhash_init_multi; the fixed version returns NULL. Regression: tests/test_init_alloc_failure.py.

#317 is reproduced with a FIFO whose writer stays open after sending 65536 bytes: the base continues waiting for more input; the fix reports the limit and exits. Regression: tests/test_template_limit.py.

#318 includes a zero-length bit-check guard. UBSan found ctz(0) and an oversized shift in the existing --missing test; the fix then passed all 26 CLI groups in the UBSan build. The PR is still Draft while the independently rooted fixes are split into issue-linked PRs.

Comment thread calc_sums.c Outdated

/* search for the sum enclosed in brackets */
for (; e >= filepath && !IS_PATH_SEPARATOR(*e); e--) {
for (pos = filepath_length - 10; ; pos--) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Since this loop is rewritten, it requires testing or thorough verification to ensure, that behaviour has not been changed. Why not leave the loop as is?

@dajiaohuang dajiaohuang changed the title Fix edge cases found in audit Remaining audit fixes pending validation and separation Oct 4, 2026
@dajiaohuang
dajiaohuang marked this pull request as draft October 4, 2026 17:50
@dajiaohuang

Copy link
Copy Markdown
Contributor Author

I have restored the original pointer-based scan loop in 5300b20. The change now only guards paths shorter than ten characters, the malformed-bracket skip, and the final decrement so they cannot form a pointer before the path buffer.

The new standalone CLI test covers 11 filename cases, including bracket/parenthesis CRCs, short names, malformed brackets, and directory boundaries. Both the current master binary and this revision produce the same expected results. All 26 full CLI test groups also pass on Linux/GCC 13.3 with SHA-NI disabled. These checks establish output compatibility, not a sanitizer reproduction of the original pointer arithmetic; the remaining batch stays Draft pending validation and separation.

@dajiaohuang

Copy link
Copy Markdown
Contributor Author

Closing this mixed Draft now that its independently rooted fixes have dedicated issue-linked PRs: #324 (issue #313), #325 (#314), #326 (#316), #327 (#317), and #328 (#318). The separate torrent-path fix in #323 has already merged.

@dajiaohuang dajiaohuang closed this Oct 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants