View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0010326libmicrohttpdinternal event looppublic2025-09-01 15:472025-09-01 15:47
Reporterarthurscchan Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Summary0010326: A logic bug discovered in mhd_str.c
DescriptionThe issue is in the internal function [`mhd_strx_to_uint32_n`](https://git.gnunet.org/libmicrohttpd2.git/tree/src/mhd2/mhd_str.c#n1516) in `src/mhd2/mhd_str.c`.
This function is intended to parse a hexadecimal string into a 32-bit unsigned integer **with a maximum character bound** (`maxlen`).
Its output is expected to **match exactly** that of the standard `mhd_strx_to_uint32` (without the bound) when `maxlen >= strlen(input)`, similar to how `mhd_strx_to_uint64` and `mhd_strx_to_uint64_n` are consistent.

However, `mhd_strx_to_uint32_n` contains an extra multiply/add operation per loop iteration, which **causes** the result to mismatch with `mhd_strx_to_uint32`. The extra `res *= 16;` and `res += (unsigned int) digit;` at the end of the while loop leads to each digit being processed **twise** in every iteration, making the result far larger than expected.

```c
while (i < maxlen && (digit = xdigittovalue (str[i])) >= 0)
{
  uint_fast32_t prev_res = res;

  res *= 16;
  if (res / 16 != prev_res)
    return 0;
  res += (unsigned int) digit;
  if (res < (unsigned int) digit)
    return 0;

  res *= 16;
  res += (unsigned int) digit;
  i++;
}
```

The corresponding 64-bit function [`mhd_strx_to_uint64_n`](https://git.gnunet.org/libmicrohttpd2.git/tree/src/mhd2/mhd_str.c#n1562) performs **one multiply/add per digit**, matching the result with `mhd_strx_to_uint64` and remaining consistent.
But `mhd_strx_to_uint32_n` and `mhd_strx_to_uint32` clearly do **not** produce consistent output, which may confuze maintainers or lead to incorrect behaviour in dependant code.

As the buggy function is marked as `MHD_Internal`, it is not meant to be invoked directly by public API users. However, incorrect parsing results could propagate into request or response processing that **relies** on this helper.
Because integer overflow handling is still guarded by `maxlen` checks and memory verification, it does **not** pose an immediate memory-safety risk.
The main risk here is **functional incorrectness**, which can break assumptions or trigger subtle bugs.
Steps To ReproduceThe PoC compares the outputs of `mhd_strx_to_uint32` vs `mhd_strx_to_uint32_n` and `mhd_strx_to_uint64` vs `mhd_strx_to_uint64_n` with several input strings.
It demonstrates that the 64-bit versions are consistent, while the 32-bit versions diverge:

```
#include <cstdio>
#include <cstdint>
#include <cstring>

extern "C" {
  #include "mhd_str.h"
}

int main() {
  const char* tests[] = {
    "0", "A", "FF", "FFFFFFFF"
  };

  for (int i = 0; i < 4; i++) {
    const char* hex = tests[i];
    uint_fast32_t v32 = 0, v32n = 0;
    uint_fast64_t v64 = 0, v64n = 0;

    mhd_strx_to_uint32 (hex, &v32);
    mhd_strx_to_uint32_n (hex, strlen(hex), &v32n);
    mhd_strx_to_uint64 (hex, &v64);
    mhd_strx_to_uint64_n (hex, strlen(hex), &v64n);

    std::printf("Hex: %-10s | 32: %-10lu | 32_n: %-20lu | 64: %-12llu | 64_n: %-12llu\n",
        hex, (unsigned long) v32, (unsigned long) v32n,
        (unsigned long long) v64, (unsigned long long) v64n);
  }
  return 0;
}
```

## Build and run the poc

```
# Clone project
git clone https://git.gnunet.org/libmicrohttpd2.git mhd2
cd mhd2

# Build project
./autogen.sh
./configure --enable-dauth --enable-md5 --enable-sha256 --enable-sha512-256 \
  --enable-bauth --enable-upgrade --enable-https --enable-messages
ASAN_OPTIONS=detect_leaks=0 make -j$(nproc)
make install
BINARY=./src/mhd2/.libs/libmicrohttpd2.a

# Build poc
clang++ -fsanitize=address -stdlib=libc++ -std=c++17 -DHAVE_CONFIG_H ../test.cpp -I"$SRC/mhd2/src/mhd2" -I"$SRC/mhd2/src/include" -I"$SRC/mhd2/src/incl_priv" -I"$SRC/mhd2/src/incl_priv/config" ./src/mhd2/.libs/libmicrohttpd2.a -o poc

# Run poc
./poc
```

### Output

```
Hex: 0 | 32: 0 | 32_n: 0 | 64: 0 | 64_n: 0
Hex: A | 32: 10 | 32_n: 170 | 64: 10 | 64_n: 10
Hex: FF | 32: 255 | 32_n: 65535 | 64: 255 | 64_n: 255
Hex: FFFFFFFF | 32: 4294967295 | 32_n: 18446744073709551615 | 64: 4294967295 | 64_n: 4294967295
```

This confirms that:

* `mhd_strx_to_uint32_n` is **not consistent** with `mhd_strx_to_uint32`.
* `mhd_strx_to_uint64_n` is consistent with `mhd_strx_to_uint64` as expected.
Additional InformationFound during the ongoing security audit carried out by Ada Logics and facilitated by OSTIF in the libmicrohttpd2 project.
TagsNo tags attached.

Activities

There are no notes attached to this issue.

Issue History

Date Modified Username Field Change
2025-09-01 15:47 arthurscchan New Issue