From c0c592288c1dfaa64c1b42d2ceb527de4e3723db Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Sat, 3 Oct 2026 04:48:48 +0800 Subject: [PATCH 1/8] Fix edge cases found in audit --- calc_sums.c | 14 ++++++++++---- common_func.c | 6 ++++-- hash_check.c | 4 ++-- librhash/hex.c | 2 +- librhash/rhash.c | 8 ++++++-- output.c | 2 +- rhash_main.c | 6 +++--- 7 files changed, 27 insertions(+), 15 deletions(-) diff --git a/calc_sums.c b/calc_sums.c index 3bc29765..8fc92cf1 100644 --- a/calc_sums.c +++ b/calc_sums.c @@ -275,11 +275,16 @@ int calc_sums(struct file_info* info) int find_embedded_crc32(file_t* file, unsigned* crc32) { const char* filepath = file_get_print_path(file, FPathUtf8 | FPathNotNull); - const char* e = filepath + strlen(filepath) - 10; + size_t filepath_length = strlen(filepath); + const char* e; + size_t pos; unsigned char raw[4]; + if (filepath_length < 10) return 0; /* search for the sum enclosed in brackets */ - for (; e >= filepath && !IS_PATH_SEPARATOR(*e); e--) { + for (pos = filepath_length - 10; ; pos--) { + e = filepath + pos; + if (IS_PATH_SEPARATOR(*e)) break; if ((*e == '[' && e[9] == ']') || (*e == '(' && e[9] == ')')) { const char* p = e + 8; for (; p > e && IS_HEX(*p); p--); @@ -289,8 +294,10 @@ int find_embedded_crc32(file_t* file, unsigned* crc32) ((unsigned)raw[2] << 8) | (unsigned)raw[3]; return 1; } - e -= 9; + if (pos < 9) break; + pos -= 9; } + if (pos == 0) break; } return 0; } @@ -402,7 +409,6 @@ static int save_torrent(struct file_info* info) int res; /* append .torrent extension to the file path */ file_t torrent_file; - file_modify_path(&torrent_file, info->file, ".torrent", FModifyAppendSuffix); res = file_modify_path(&torrent_file, info->file, ".torrent", FModifyAppendSuffix); if (res >= 0) res = save_torrent_to(&torrent_file, info->rctx); diff --git a/common_func.c b/common_func.c index 04b666ed..0f626b95 100644 --- a/common_func.c +++ b/common_func.c @@ -92,8 +92,10 @@ char* str_tolower(const char* str) */ char* str_trim(char* str) { - char* last = str + strlen(str) - 1; + char* last; while (isspace((unsigned char)*str)) str++; + if (!*str) return str; + last = str + strlen(str) - 1; while (isspace((unsigned char)*last) && last > str) *(last--) = 0; return str; } @@ -193,7 +195,7 @@ char* str_replace_n(const char* src, size_t start_pos, size_t end_pos, const cha int is_binary_string(const char* str) { for (; *str; str++) { - if (((unsigned char)*str) < 32 && ((1 << (unsigned char)*str) & ~0x2600)) { + if (((unsigned char)*str) < 32 && ((1u << (unsigned char)*str) & ~0x2600u)) { return 1; } } diff --git a/hash_check.c b/hash_check.c index 8b13dc1d..95a746fe 100644 --- a/hash_check.c +++ b/hash_check.c @@ -252,7 +252,7 @@ static int is_acceptable_bit_length(int length) { int pow = get_ctz(length >> 5); int code = ((length >> (pow + 6)) << 3) | pow; - return (code < 32 && ((1 << code) & 0x101061d)); + return (code < 32 && ((1u << code) & 0x101061du)); } return 0; } @@ -347,7 +347,7 @@ static unsigned bsd_hash_name_to_id(const char* name, size_t length, enum HashNa return (length == strlen(hash_info_table[i].name) ? hash_mask : 0); } /* look for the hash_id in the hash_mask */ - for (hash_id = 1 << i; hash_id && hash_id <= hash_mask; i++, hash_id <<= 1) + for (hash_id = 1u << i; hash_id && hash_id <= hash_mask; i++, hash_id <<= 1) { const char* a; const char* b; diff --git a/librhash/hex.c b/librhash/hex.c index b0119045..ebc793fa 100644 --- a/librhash/hex.c +++ b/librhash/hex.c @@ -134,7 +134,7 @@ size_t rhash_base64_url_encoded_helper(char* dst, const unsigned char* src, size /* RFC 3986: safe url characters are ascii alpha-numeric and "-._~", other characters should be percent-encoded */ static unsigned url_safe_char_mask[4] = { 0, 0x03ff6000, 0x87fffffe, 0x47fffffe }; -#define IS_URL_GOOD_CHAR(c) ((unsigned)(c) < 128 && (url_safe_char_mask[c >> 5] & (1 << (c & 31)))) +#define IS_URL_GOOD_CHAR(c) ((unsigned)(c) < 128 && (url_safe_char_mask[c >> 5] & (1u << (c & 31)))) /** * URL-encode specified binary string. diff --git a/librhash/rhash.c b/librhash/rhash.c index 56c4aee5..80d09020 100644 --- a/librhash/rhash.c +++ b/librhash/rhash.c @@ -158,6 +158,7 @@ static rhash_context_ext* rhash_alloc_multi(size_t count, const unsigned hash_id RHASH_API rhash rhash_init_multi(size_t count, const unsigned hash_ids[]) { rhash_context_ext* ectx = rhash_alloc_multi(count, hash_ids, 1); + if (!ectx) return NULL; return &ectx->rc; /* return initialized rhash context */ } @@ -1057,8 +1058,11 @@ static unsigned ids_array_to_hash_bitmask(size_t count, unsigned* data) bitmask |= data[i]; else if (data[i] == RHASH_ALL_HASHES) bitmask |= RHASH_LOW_HASHES_MASK; - else - bitmask |= 1 << GET_EXTENDED_HASH_ID_INDEX(data[i]); + else { + unsigned index = GET_EXTENDED_HASH_ID_INDEX(data[i]); + if (index < RHASH_HASH_COUNT) + bitmask |= 1u << index; + } } return bitmask; } diff --git a/output.c b/output.c index 813a97de..fe4c5a41 100644 --- a/output.c +++ b/output.c @@ -173,7 +173,7 @@ int fprintf_file_t(FILE* out, const char* format, struct file_t* file, unsigned /* RFC 3986: safe url characters are ascii alpha-numeric and "-._~", other characters should be percent-encoded */ static unsigned url_safe_char_mask[4] = { 0, 0x03ff6000, 0x87fffffe, 0x47fffffe }; -#define IS_URL_GOOD_CHAR(c) ((unsigned)(c) < 128 && (url_safe_char_mask[c >> 5] & (1 << (c & 31)))) +#define IS_URL_GOOD_CHAR(c) ((unsigned)(c) < 128 && (url_safe_char_mask[c >> 5] & (1u << (c & 31)))) /** * Print to a stram an url-encoded representation of the given string. diff --git a/rhash_main.c b/rhash_main.c index 264040a1..a35b74bf 100644 --- a/rhash_main.c +++ b/rhash_main.c @@ -168,12 +168,12 @@ static int load_printf_template(void) while (!feof(fd)) { len = fread(buffer, 1, 8192, fd); if (ferror(fd)) break; - - rsh_str_append_n(rhash_data.template_text, buffer, len); - if (rhash_data.template_text->len >= MAX_TEMPLATE_SIZE) { + if (len >= MAX_TEMPLATE_SIZE - rhash_data.template_text->len) { log_msg_file_t(_("%s: template file is too big\n"), &file); error = 1; + break; } + rsh_str_append_n(rhash_data.template_text, buffer, len); } if (ferror(fd)) { From e1d8d3ca3217b022e54f29c91e4b0580ca8fe34a Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Mon, 5 Oct 2026 01:50:46 +0800 Subject: [PATCH 2/8] chore: separate torrent path allocation fix into its own PR --- calc_sums.c | 1 + 1 file changed, 1 insertion(+) diff --git a/calc_sums.c b/calc_sums.c index 8fc92cf1..0b2e6490 100644 --- a/calc_sums.c +++ b/calc_sums.c @@ -409,6 +409,7 @@ static int save_torrent(struct file_info* info) int res; /* append .torrent extension to the file path */ file_t torrent_file; + file_modify_path(&torrent_file, info->file, ".torrent", FModifyAppendSuffix); res = file_modify_path(&torrent_file, info->file, ".torrent", FModifyAppendSuffix); if (res >= 0) res = save_torrent_to(&torrent_file, info->rctx); From 5300b20d2f859f8d8ed76dab0df8f686b20e5752 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Mon, 5 Oct 2026 02:00:39 +0800 Subject: [PATCH 3/8] Preserve embedded CRC scan loop and test filename boundaries --- calc_sums.c | 12 +++++------- tests/test_embedded_crc_bounds.py | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) create mode 100644 tests/test_embedded_crc_bounds.py diff --git a/calc_sums.c b/calc_sums.c index 0b2e6490..afbbcb79 100644 --- a/calc_sums.c +++ b/calc_sums.c @@ -277,14 +277,12 @@ int find_embedded_crc32(file_t* file, unsigned* crc32) const char* filepath = file_get_print_path(file, FPathUtf8 | FPathNotNull); size_t filepath_length = strlen(filepath); const char* e; - size_t pos; unsigned char raw[4]; if (filepath_length < 10) return 0; + e = filepath + filepath_length - 10; /* search for the sum enclosed in brackets */ - for (pos = filepath_length - 10; ; pos--) { - e = filepath + pos; - if (IS_PATH_SEPARATOR(*e)) break; + for (; e >= filepath && !IS_PATH_SEPARATOR(*e); e--) { if ((*e == '[' && e[9] == ']') || (*e == '(' && e[9] == ')')) { const char* p = e + 8; for (; p > e && IS_HEX(*p); p--); @@ -294,10 +292,10 @@ int find_embedded_crc32(file_t* file, unsigned* crc32) ((unsigned)raw[2] << 8) | (unsigned)raw[3]; return 1; } - if (pos < 9) break; - pos -= 9; + if (e - filepath < 10) break; + e -= 9; } - if (pos == 0) break; + if (e == filepath) break; } return 0; } diff --git a/tests/test_embedded_crc_bounds.py b/tests/test_embedded_crc_bounds.py new file mode 100644 index 00000000..692d08df --- /dev/null +++ b/tests/test_embedded_crc_bounds.py @@ -0,0 +1,25 @@ +#!/usr/bin/env python3 +from pathlib import Path +import subprocess +import sys +import tempfile +import zlib + +binary = str(Path(sys.argv[1]).resolve()) +crc = f"{zlib.crc32(b"A"):08X}" +cases = [("a", False), ("123456789", False), ("1234567890", False), + (f"[{crc}]", True), (f"({crc})", True), + (f"prefix_[{crc}].data", True), (f"[{crc}][ZZZZZZZZ]", True), + ("[ZZZZZZZZ]", False), ("x[ZZZZZZZZ]", False), + (f"[{crc}]/plain.data", False), (f"nested/[{crc}].data", True)] +with tempfile.TemporaryDirectory() as directory: + for index, (name, expected) in enumerate(cases): + case_directory = Path(directory, str(index)) + path = case_directory / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(b"A") + result = subprocess.run([binary, "--check-embedded", name], cwd=case_directory, + capture_output=True, text=True, timeout=10) + successful = "OK" in result.stdout and result.returncode == 0 + assert successful == expected, (name, result.returncode, result.stdout, result.stderr) +print(f"PASS: {len(cases)} embedded CRC filename cases") From 1fbd3d6a0fb55e879b9599c3c9bf47c753e590ae Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Mon, 5 Oct 2026 14:44:20 +0800 Subject: [PATCH 4/8] Test empty option names in configuration --- tests/test_empty_config_name.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 tests/test_empty_config_name.py diff --git a/tests/test_empty_config_name.py b/tests/test_empty_config_name.py new file mode 100644 index 00000000..ee45aaed --- /dev/null +++ b/tests/test_empty_config_name.py @@ -0,0 +1,19 @@ +#!/usr/bin/env python3 +from pathlib import Path +import os +import subprocess +import sys +import tempfile + +binary = str(Path(sys.argv[1]).resolve()) +with tempfile.TemporaryDirectory() as directory: + config = Path(directory, "rhash", "rhashrc") + config.parent.mkdir() + config.write_text("=ignored\nsha256=on\n") + env = dict(os.environ, XDG_CONFIG_HOME=directory) + result = subprocess.run([binary, "--message=abc"], env=env, + capture_output=True, text=True, timeout=10) + assert result.returncode == 0, (result.returncode, result.stderr) + assert "unknown option" in result.stderr, result.stderr + assert "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" in result.stdout, result.stdout +print("PASS: empty configuration name warns and subsequent SHA-256 option works") From b2eb43c03a055a3f74f4efd1a56a0847903f95c3 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Mon, 5 Oct 2026 14:49:00 +0800 Subject: [PATCH 5/8] Test short embedded CRC filenames --- tests/test_embedded_crc_short_paths.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 tests/test_embedded_crc_short_paths.py diff --git a/tests/test_embedded_crc_short_paths.py b/tests/test_embedded_crc_short_paths.py new file mode 100644 index 00000000..40b5dc26 --- /dev/null +++ b/tests/test_embedded_crc_short_paths.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +import os +from pathlib import Path +import subprocess +import sys +import tempfile + +binary = str(Path(sys.argv[1]).resolve()) +env = dict(os.environ, ASAN_OPTIONS="detect_invalid_pointer_pairs=2") +with tempfile.TemporaryDirectory() as directory: + Path(directory, "a").write_bytes(b"A") + result = subprocess.run([binary, "--check-embedded", "a"], cwd=directory, + env=env, capture_output=True, text=True, timeout=10) + assert "file name doesn't contain a CRC32" in result.stderr, result.stderr + assert "invalid-pointer-pair" not in result.stderr, result.stderr +print("PASS: short filename is rejected without invalid pointer comparison") From 41b847c0c003666093cbc9460fe833142c3fc374 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Mon, 5 Oct 2026 14:49:27 +0800 Subject: [PATCH 6/8] Assert embedded CRC error on short paths --- tests/test_embedded_crc_short_paths.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_embedded_crc_short_paths.py b/tests/test_embedded_crc_short_paths.py index 40b5dc26..90119324 100644 --- a/tests/test_embedded_crc_short_paths.py +++ b/tests/test_embedded_crc_short_paths.py @@ -11,6 +11,7 @@ Path(directory, "a").write_bytes(b"A") result = subprocess.run([binary, "--check-embedded", "a"], cwd=directory, env=env, capture_output=True, text=True, timeout=10) + assert result.returncode == 1, (result.returncode, result.stderr) assert "file name doesn't contain a CRC32" in result.stderr, result.stderr assert "invalid-pointer-pair" not in result.stderr, result.stderr print("PASS: short filename is rejected without invalid pointer comparison") From f546ab42f06dcd1baecfc6ff43f4668537354db3 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Mon, 5 Oct 2026 14:58:36 +0800 Subject: [PATCH 7/8] Guard allocation and zero-length bit checks --- hash_check.c | 2 +- tests/test_init_alloc_failure.py | 60 ++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 tests/test_init_alloc_failure.py diff --git a/hash_check.c b/hash_check.c index 95a746fe..38de4920 100644 --- a/hash_check.c +++ b/hash_check.c @@ -248,7 +248,7 @@ static int detect_hash_type(char** ptr, char* end, int* p_len) */ static int is_acceptable_bit_length(int length) { - if ((length & 31) == 0 && length <= 512) + if (length > 0 && (length & 31) == 0 && length <= 512) { int pow = get_ctz(length >> 5); int code = ((length >> (pow + 6)) << 3) | pow; diff --git a/tests/test_init_alloc_failure.py b/tests/test_init_alloc_failure.py new file mode 100644 index 00000000..fe3a732c --- /dev/null +++ b/tests/test_init_alloc_failure.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python3 +import os +from pathlib import Path +import subprocess +import tempfile + +root = Path(__file__).resolve().parents[1] +library = next((root / "librhash").glob("librhash.so.*")) +with tempfile.TemporaryDirectory() as directory: + directory = Path(directory) + hook = directory / "fail_alloc.c" + driver = directory / "check_init.c" + hook.write_text(r'''#define _GNU_SOURCE +#include +#include +#include +static int fail_next; +static int failed_count; +static void* (*real_aligned_alloc)(size_t, size_t); +__attribute__((constructor)) static void resolve_aligned_alloc(void) { + real_aligned_alloc = dlsym(RTLD_NEXT, "aligned_alloc"); +} +void rhash_test_fail_next_aligned_alloc(void) { fail_next = 1; } +int rhash_test_failed_count(void) { return failed_count; } +void* aligned_alloc(size_t alignment, size_t size) { + if (fail_next) { fail_next = 0; failed_count++; errno = ENOMEM; return NULL; } + return real_aligned_alloc(alignment, size); +} +''') + driver.write_text(r'''#include +#include +#include +int main(void) { + unsigned id = RHASH_SHA256; + void (*fail_next)(void) = dlsym(RTLD_DEFAULT, "rhash_test_fail_next_aligned_alloc"); + int (*failed_count)(void) = dlsym(RTLD_DEFAULT, "rhash_test_failed_count"); + rhash ctx; + if (!fail_next || !failed_count) return 2; + rhash_library_init(); + fail_next(); + ctx = rhash_init_multi(1, &id); + if (ctx) { rhash_free(ctx); return 3; } + if (failed_count() != 1) return 4; + puts("PASS: allocation failure returns NULL"); + return 0; +} +''') + cc = os.environ.get("CC", "cc") + subprocess.run([cc, "-shared", "-fPIC", str(hook), "-ldl", "-o", str(directory / "fail_alloc.so")], check=True) + subprocess.run([cc, str(driver), "-I", str(root / "librhash"), str(library), "-ldl", + f"-Wl,-rpath,{root / 'librhash'}", "-o", str(directory / "check_init")], check=True) + env = dict(os.environ, LD_PRELOAD=str(directory / "fail_alloc.so"), + LD_LIBRARY_PATH=str(root / "librhash"), + UBSAN_OPTIONS="halt_on_error=1:print_stacktrace=1") + result = subprocess.run([str(directory / "check_init")], env=env, capture_output=True, + text=True, timeout=10) + assert result.returncode == 0, (result.returncode, result.stdout, result.stderr) + assert "PASS: allocation failure returns NULL" in result.stdout, result.stdout + assert "runtime error:" not in result.stderr, result.stderr +print("PASS: rhash_init_multi handles aligned-allocation failure") From 2fb371fddfb32bb8d8ee5d3c2f38d7e7315fe5d5 Mon Sep 17 00:00:00 2001 From: dajiaohuang Date: Mon, 5 Oct 2026 15:01:13 +0800 Subject: [PATCH 8/8] Test template limit and compatible CRC fixtures --- tests/test_embedded_crc_bounds.py | 3 ++- tests/test_template_limit.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 tests/test_template_limit.py diff --git a/tests/test_embedded_crc_bounds.py b/tests/test_embedded_crc_bounds.py index 692d08df..3863aee0 100644 --- a/tests/test_embedded_crc_bounds.py +++ b/tests/test_embedded_crc_bounds.py @@ -6,7 +6,8 @@ import zlib binary = str(Path(sys.argv[1]).resolve()) -crc = f"{zlib.crc32(b"A"):08X}" +crc_value = zlib.crc32(b"A") +crc = f"{crc_value:08X}" cases = [("a", False), ("123456789", False), ("1234567890", False), (f"[{crc}]", True), (f"({crc})", True), (f"prefix_[{crc}].data", True), (f"[{crc}][ZZZZZZZZ]", True), diff --git a/tests/test_template_limit.py b/tests/test_template_limit.py new file mode 100644 index 00000000..fb29ede3 --- /dev/null +++ b/tests/test_template_limit.py @@ -0,0 +1,28 @@ +#!/usr/bin/env python3 +import os +from pathlib import Path +import subprocess +import sys +import tempfile + +binary = str(Path(sys.argv[1]).resolve()) +with tempfile.TemporaryDirectory() as directory: + template = Path(directory, "template.fmt") + os.mkfifo(template) + process = subprocess.Popen([binary, f"--template={template}", "--message=abc"], + cwd=directory, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + text=True) + with template.open("wb", buffering=0) as writer: + try: + writer.write(b"x" * 65536) + except BrokenPipeError: + pass + try: + stdout, stderr = process.communicate(timeout=2) + except subprocess.TimeoutExpired: + process.kill() + stdout, stderr = process.communicate() + raise AssertionError("RHash kept reading the template after the size limit") + assert process.returncode == 2, (process.returncode, stdout, stderr) + assert "template file is too big" in stderr, stderr +print("PASS: template loading stops at the configured size limit")