[SECURITY] Heap Buffer Overflow in mosquitto_property_add_binary due to unconstrained input length

[xt-cve]

Description

Fuzz testing on the mosquitto_property_add_binary function has revealed a Heap Buffer Overflow when reading data from the fuzzer input buffer.

The vulnerability stems from the fuzzer harness reading a uint16_t length (len) directly from the fuzzer input, but this len value is not constrained by the total available fuzzer input size (Size). This leads to an out-of-bounds read (READ of size 37167) from the fuzzer's memory buffer during the memcpy operation inside mosquitto_property_add_binary.

Vulnerability Details

1. Crash Backtrace (ASAN Output) The AddressSanitizer (ASAN) report clearly shows a multi-byte read overflow:

=================================================================
==12==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x50200001b716 at pc 0x55910caa7fa2 bp 0x7ffde20a7710 sp 0x7ffde20a6ed0
READ of size 37167 at 0x50200001b716 thread T0
SCARINESS: 26 (multi-byte-read-heap-buffer-overflow)
    #0 0x55910caa7fa1 in __asan_memcpy /src/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:63:3
    #1 0x55910caeb9c6 in mosquitto_property_add_binary /src/mosquitto/libcommon/property_common.c:557:3  <-- Crash location
    #2 0x55910cae988e in LLVMFuzzerTestOneInput /src/mosquitto/fuzzing/libcommon/libcommon_fuzz_pub_topic_check2.cpp:38:5
    ... (rest of the stack)

2. Code Analysis The bug is introduced in the fuzzer harness (LLVMFuzzerTestOneInput) and then exploited in the target function (mosquitto_property_add_binary).

Fuzzer Harness Logic (libcommon_fuzz_pub_topic_check2.cpp):

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
    if (Size < 6) {
        return 0;
    }

    int identifier = *(int *)(Data);
    uint16_t len = *(uint16_t *)(Data + 4); // Potentially large, unconstrained value
    const void *value = Data + 6;

    mosquitto_property *proplist = NULL;

    // If len is greater than (Size - 6), the following call will cause an overflow.
    mosquitto_property_add_binary(&proplist, identifier, value, len);
    // ...
}

Target Function Logic (mosquitto_property_add_binary):

The memcpy in the target function is the point of memory corruption:

// property_common.c:557
    if(len){
        prop->value.bin.v = mosquitto__malloc(len); // Malloc succeeds for large 'len'
        if(!prop->value.bin.v){
            mosquitto__free(prop);
            return MOSQ_ERR_NOMEM;
        }

        memcpy(prop->value.bin.v, value, len); // <-- Out-of-bounds READ of 'value'
        prop->value.bin.len = len;
    }

When len (a uint16_t from the fuzzer input) is large (e.g., 37167 as seen in the ASAN output) but the total fuzzer input size (Size) is much smaller, the memcpy attempts to read len bytes starting from value (which is Data + 6), reading far beyond the end of the Data buffer.

Suggested Fix

Review mosquitto_property_add_binary (Defense in Depth)

It is recommended to review how mosquitto_property_add_binary (and other property-adding functions) are used across the codebase, ensuring that all callers validate the size of the value buffer against the provided len.

[ralight]

If I understand this correctly, you've written a buggy fuzzer and think this is a security error in the library? The whole point of the len argument is for the caller of the function to correctly indicate the length of the buffer being passed. There's no way for the function to know the size of the buffer, if there was the len argument would be redundant.