4 ms·

> It is a valid bug. But is it a high-severity security bug?

• Quekid5 1y ago
  Considering that it's Undefined Behavior, quite possibly. EDIT: That said, I'm on the maintainer's side here.
  • gpderetta 1y ago
    > 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.
    • ynik 1y ago
      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.
      • gpderetta 1y ago
        Actually you are right, what I said about reordering is nonsense. The compiler will definitely reorder non-aliasing accesses. There are much weaker properties that are preserved.
      • somat 1y ago
        I am not so sure. Assuming the program does not do anything undefined is sort of the worst possible take on leaving the behavior undefined in the first place. I mean the behavior was left undefined so that "something" could be done. the language standard just does not know what that "something" is. Hell the compiler could do nothing and that would make more sense. But to make optimizations pretending it is an invariant, it can't happen, when the specification clearly says it could happen. That's wild, and I would argue out of specification.
        • Quekid5 1y ago
          Undefined Behavior has a very specific meaning in the C and (respectively) C++ standards. And it is this. I think you may be misunderstand how optimization works. It's not "AHA! Undefined Behavior, I'll poke at that!". It's more that compilers are allowed to assume that some things just "don't happen"... and can optimize based on that. And it does make sense... if you were told to optimize a thing given certain rules, how would you do it?
• charleslmunger 1y ago
  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.