Hacker News Reader

Hacker News

Comment by endorphine | Hacker News Reader

Parent Full thread

endorphine

Is this a vulnerability that would be impossible kn6, let's say, Rust?

View on HN

This seems to be the patch: https://github.com/github/cmark-gfm/commit/cf7577d2f74289cb8...

Integer overflow can happen in Rust, but it's well-defined, not undefined. This helps.

Bounds checking is part of indexing, and so even if an index overflows, the check should happen, and panic.

"impossible" is a strong word, but it would be significantly less likely in Rust. If you did the same thing as you did in C, with unsafe, then it could happen. But there's not a lot of reason to 99.9999% of the time, as it's the more difficult and less ergonomic option.

the actual commit fix has some comments that may be useful for understanding:

https://github.com/github/cmark-gfm/commit/ac80f7b56522ffa15...

I’m in my phone now but they cut two different patches to two different releases, I suspect I linked to one and you the other. Harder to double check that when I’m not at a computer, though that does have far better comments and I should have linked to it, thank you. I basically picked one at random.

no worries, it's the same patch to two different releases; you linked to the merge commit & I linked to the fix commit.

source: i cut the releases ;)

Ah ha! I should have realized. Thank you for your hard work, this kind of thing is never easy.

Is this unsigned integer overflow? Isn’t that well defined in C++ as well?

Edit: I didn’t research where the corruption comes from in this bug.

Edit again: it looks like the source file is actually C and not C++.

Yep, well-defined in C++, but the resulting out-of-bounds accesses and all that are not well-defined.

Thanks. I should look at the code. I thought unsigned int overflow would wrap to zero, which would still be in bounds for a nontrivial array. Maybe they’re freeing the item at that index the first time through the array or something.

Well defined and it also panics in debug mode. Unit tests tend not to catch these sorts of bugs tbh, but still, nice to have :)

Yes. Heap Memory Corruption is a type of memory safety issue that's impossible in Safe Rust. (As usual, this depends on any unsafe code and the compiler being bug-free, but that's supposed to be much easier to prove since the "scope" of things to check for correctness is much reduced).

Rust programs don't call `malloc` directly, so the problem of overflow in malloc size calculation is mitigated by never needing to write such code (Rust programs use something like Vec, which is a safe abstraction that reliably (re)allocates as much as required.)

Rust's lack of implicit numeric conversions pushes authors towards using usize (size_t) for everything. So in Rust you'd be more likely to have a denial of service due to supporting 2^64 columns. If you tried to carelessly use u16 for the number of columns, you'd more likely have an application level bug like incorrect page rendering, or in the worst case a panic (equivalent of an uncaught C++ exception, which may be a program-stopping bug, but not a vulnerability).

Unexpected overflow faults in most modern safe languages (rust, swift, presumably go?) by default - they generally use different operators or functions for when overflow is ok.

Reply on news.ycombinator.com