← Back to context

Comment by saurik

2 months ago

> It is a valid bug.

But is it a high-severity security bug?

If there was another bug that allowed input data to request a large allocation, this would be?

Normally I'd want a secure parser to allocate memory with a bound, and one way to implement that is to give each parse call an allocator that fails once it reaches a limit. From a very basic scan it seems like libxml2 supports swapping its allocator function, so it could do this; although that's probably not the intended purpose since those APIs are global rather than per parser instance.

I bet what happened here is the reporter ran a fuzzer, found a report, and didn't realize that the fuzzer configuration was injecting faults into malloc so they thought it was more severe than it actually was. Missing a bounds check often results in some massive number being passed to malloc, which then returns null, so this is a common pattern to find. It's not a high severity security bug but it's reasonable to think it could be one if you didn't know about the fault injecting allocator.

Considering that it's Undefined Behavior, quite possibly.

EDIT: That said, I'm on the maintainer's side here.

  • > Considering that it's Undefined Behavior, quite possibly.

    Is it thought? Certainly it is according the C and C++ standards, but POSIX adds:

    > References to unmapped addresses shall result in a SIGSEGV signal

    While time-traveling-UB is a theoretical possibility, practically POSIX compliant compilers won't reorder around potentially trapping operations (they will do the reverse, they might remove a null check if made redundant by a prior potentially trapping dereference) .

    A real concern is if a null pointer is dereferenced with a large attacker-controlled offset that can avoid the trap, but that's more of an issue of failing to bound check.

    • Under your interpretation, neither gcc nor clang are POSIX compliant. Because in practice all these optimizing compilers will reorder memory accesses without bothering to prove that the pointers involved are valid -- the compiler just assumes that the pointers are valid, which is justified because otherwise the program would have undefined behavior.

      3 replies →