Remaining audit fixes pending validation and separation

calc_sums.c

@@ -275,8 +275,11 @@ 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;
 	unsigned char raw[4];
+	if (filepath_length < 10) return 0;
+	e = filepath + filepath_length - 10;
 
 	/* search for the sum enclosed in brackets */
 	for (; e >= filepath && !IS_PATH_SEPARATOR(*e); e--) {
@@ -289,8 +292,10 @@ int find_embedded_crc32(file_t* file, unsigned* crc32)
 					((unsigned)raw[2] << 8) | (unsigned)raw[3];
 				return 1;
 			}
+			if (e - filepath < 10) break;
 			e -= 9;
 		}
+		if (e == filepath) break;
 	}
 	return 0;
 }

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;
 		}
 	}

hash_check.c

@@ -248,11 +248,11 @@ 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;
-		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;

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.

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;
 }

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.

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)) {

tests/test_embedded_crc_bounds.py

@@ -0,0 +1,26 @@
+#!/usr/bin/env python3
+from pathlib import Path
+import subprocess
+import sys
+import tempfile
+import zlib
+
+binary = str(Path(sys.argv[1]).resolve())
+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),
+         ("[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")

tests/test_embedded_crc_short_paths.py

@@ -0,0 +1,17 @@
+#!/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 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")

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")

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 <dlfcn.h>
+#include <errno.h>
+#include <stddef.h>
+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 <dlfcn.h>
+#include <stdio.h>
+#include <rhash.h>
+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")

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")