View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0010326libmicrohttpd2Generalpublic2025-09-01 15:472025-09-07 01:01
Reporterarthurscchan Assigned ToKarlson2k  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Fixed in VersionGit master 
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

Karlson2k

2025-09-06 23:12

manager   ~0025848

The function mhd_strx_to_uint32_n() was unused, so the bug is never triggered.

However, the possible future issue has been fixed by 8d8ccb4b32810fec690ccb99c879c361de8c26db.

Issue History

Date Modified Username Field Change
2025-09-01 15:47 arthurscchan New Issue
2025-09-06 23:10 Karlson2k Assigned To => Karlson2k
2025-09-06 23:10 Karlson2k Status new => confirmed
2025-09-06 23:12 Karlson2k Status confirmed => resolved
2025-09-06 23:12 Karlson2k Resolution open => fixed
2025-09-06 23:12 Karlson2k Fixed in Version => Git master
2025-09-06 23:12 Karlson2k Note Added: 0025848
2025-09-07 01:01 root Project libmicrohttpd => libmicrohttpd2
2025-09-07 01:01 root Category internal event loop => General