fix: signed-shift overflow in hdr_calculate_bucket_config (UBSan, found by fuzzing)

.github/workflows/ci.yml

@@ -69,3 +69,23 @@ jobs:
         run: cmake --build _build --config ${{ matrix.build_type }}
       - name: Test
         run: cmake -E chdir _build ctest --build-config ${{ matrix.build_type }}
+
+  # Run the unit suite under AddressSanitizer + UndefinedBehaviorSanitizer so
+  # memory-safety and integer-overflow regressions (the class the fuzzers find)
+  # are caught deterministically per-PR, not only in the weekly fuzzing run.
+  sanitizers:
+    name: sanitizers (linux, Debug, ASan+UBSan)
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v1
+        with:
+          submodules: recursive
+      - name: Install dependencies
+        run: sudo apt-get update && sudo apt-get install -y zlib1g-dev
+      - name: Configure
+        run: cmake -E make_directory _build && cmake -E chdir _build cmake .. -DCMAKE_BUILD_TYPE=Debug -DHDR_LOG_REQUIRED=ON -DCMAKE_C_FLAGS="-fsanitize=address,undefined -fno-sanitize-recover=all -g"
+      - name: Build
+        run: cmake --build _build
+      - name: Test
+        run: cmake -E chdir _build ctest --output-on-failure

.gitignore

@@ -54,3 +54,4 @@ debug
 cmake-build-debug
 /build-afl
 .vscode
+_build

src/hdr_histogram.c

@@ -392,9 +392,14 @@ int hdr_calculate_bucket_config(
     int32_t sub_bucket_count_magnitude;
     int64_t largest_value_with_single_unit_resolution;
 
+    /* define cfg on every reject path so a two-step-init caller that mishandles
+       the EINVAL return never reads uninitialized fields */
+    memset(cfg, 0, sizeof(*cfg));
+
     if (lowest_discernible_value < 1 ||
             significant_figures < 1 || 5 < significant_figures ||
-            lowest_discernible_value * 2 > highest_trackable_value)
+            /* division form: lowest*2 near INT64_MAX overflows int64 (UB) */
+            lowest_discernible_value > highest_trackable_value / 2)
     {
         return EINVAL;
     }
@@ -416,13 +421,15 @@ int hdr_calculate_bucket_config(
     cfg->unit_magnitude = (int32_t) unit_magnitude;
     cfg->sub_bucket_count      = (int32_t) pow(2, (cfg->sub_bucket_half_count_magnitude + 1));
     cfg->sub_bucket_half_count = cfg->sub_bucket_count / 2;
-    cfg->sub_bucket_mask       = ((int64_t) cfg->sub_bucket_count - 1) << cfg->unit_magnitude;
 
+    /* reject before shifting: sub_bucket_mask shift past bit 61 is signed-shift UB */
     if (cfg->unit_magnitude + cfg->sub_bucket_half_count_magnitude > 61)
     {
         return EINVAL;
     }
 
+    cfg->sub_bucket_mask       = ((int64_t) cfg->sub_bucket_count - 1) << cfg->unit_magnitude;
+
     cfg->bucket_count = buckets_needed_to_cover_value(highest_trackable_value, cfg->sub_bucket_count, (int32_t)cfg->unit_magnitude);
     cfg->counts_len = (cfg->bucket_count + 1) * (cfg->sub_bucket_count / 2);
 

src/hdr_histogram_log.c

@@ -451,7 +451,7 @@ static int hdr_decode_compressed_v0(
     else
     {
         hdr_add(*histogram, h);
-        hdr_free(h);
+        hdr_close(h); /* hdr_close: counts is a separate alloc; hdr_free leaks it */
     }
 
     return result;
@@ -556,7 +556,7 @@ static int hdr_decode_compressed_v1(
     else
     {
         hdr_add(*histogram, h);
-        hdr_free(h);
+        hdr_close(h);
     }
 
     return result;
@@ -658,7 +658,7 @@ static int hdr_decode_compressed_v2(
     else
     {
         hdr_add(*histogram, h);
-        hdr_free(h);
+        hdr_close(h);
     }
 
     return result;

test/hdr_histogram_atomic_concurrency_test.c

@@ -50,6 +50,7 @@ static char* test_recording_concurrently(void)
     struct hdr_iter expected_iter;
     struct hdr_iter actual_iter;
     pthread_t threads[2];
+    char* result;
     int i;
 
     mu_assert("init", 0 == hdr_init(1, 10000000, 2, &expected_histogram));
@@ -82,7 +83,13 @@ static char* test_recording_concurrently(void)
     hdr_iter_init(&expected_iter, expected_histogram);
     hdr_iter_init(&actual_iter, actual_histogram);
 
-    return compare_histograms(expected_histogram, actual_histogram);
+    result = compare_histograms(expected_histogram, actual_histogram);
+
+    free(values); /* plain array */
+    hdr_close(expected_histogram);
+    hdr_close(actual_histogram);
+
+    return result;
 }
 
 static struct mu_result all_tests(void)

test/hdr_histogram_atomic_test.c

@@ -45,28 +45,28 @@ static void load_histograms(void)
     int i;
     if (raw_histogram)
     {
-        free(raw_histogram);
+        hdr_close(raw_histogram); /* hdr_close: counts is a separate alloc; free() leaks it */
     }
 
     hdr_init(1, highest_trackable_value, significant_figures, &raw_histogram);
 
     if (cor_histogram)
     {
-        free(cor_histogram);
+        hdr_close(cor_histogram);
     }
 
     hdr_init(1, highest_trackable_value, significant_figures, &cor_histogram);
 
     if (scaled_raw_histogram)
     {
-        free(scaled_raw_histogram);
+        hdr_close(scaled_raw_histogram);
     }
 
     hdr_init(1000, highest_trackable_value * 512, significant_figures, &scaled_raw_histogram);
 
     if (scaled_cor_histogram)
     {
-        free(scaled_cor_histogram);
+        hdr_close(scaled_cor_histogram);
     }
 
     hdr_init(1000, highest_trackable_value * 512, significant_figures, &scaled_cor_histogram);
@@ -96,7 +96,7 @@ static char* test_create(void)
     mu_assert("Failed to allocate hdr_histogram", h != NULL);
     mu_assert("Incorrect array length", compare_int64(h->counts_len, 23552));
 
-    free(h);
+    hdr_close(h);
 
     return 0;
 }
@@ -127,6 +127,8 @@ static char* test_create_with_large_values(void)
         "99.0% Percentile",
         hdr_values_are_equivalent(h, 100000000, hdr_value_at_percentile(h, 99.0)));
 
+    hdr_close(h);
+
     return 0;
 }
 
@@ -438,6 +440,8 @@ static char* test_out_of_range_values(void)
     mu_assert("Should successfully record value", hdr_record_value_atomic(h, 1000));
     mu_assert("Should not record value", !hdr_record_value_atomic(h, 1001));
 
+    hdr_close(h);
+
     return 0;
 }
 
@@ -474,6 +478,8 @@ static char* test_linear_iter_buckets_correctly(void)
     mu_assert("Number of steps", compare_int64(4, step_count));
     mu_assert("Total count", compare_int64(6, total_count));
 
+    hdr_close(h);
+
     return 0;
 }
 
@@ -508,17 +514,20 @@ static char* test_interval_recording(void)
     result = compare_histograms(expected_histogram, recorder_histogram);
     if (result)
     {
-        return result;
+        goto cleanup;
     }
 
     recorder_corrected_histogram = hdr_interval_recorder_sample(&recorder_corrected);
     result = compare_histograms(expected_corrected_histogram, recorder_corrected_histogram);
-    if (result)
-    {
-        return result;
-    }
 
-    return 0;
+cleanup:
+    /* destroy closes recorder active+inactive (incl. sampled histograms) */
+    hdr_close(expected_histogram);
+    hdr_close(expected_corrected_histogram);
+    hdr_interval_recorder_destroy(&recorder);
+    hdr_interval_recorder_destroy(&recorder_corrected);
+
+    return result;
 }
 
 static struct mu_result all_tests(void)
@@ -547,6 +556,12 @@ static int hdr_histogram_run_tests(void)
 {
     struct mu_result result = all_tests();
 
+    /* free static fixtures (hdr_close is NULL-safe) */
+    hdr_close(raw_histogram);
+    hdr_close(cor_histogram);
+    hdr_close(scaled_raw_histogram);
+    hdr_close(scaled_cor_histogram);
+
     if (result.message != 0)
     {
         printf("hdr_histogram_test.%s(): %s\n", result.test, result.message);