Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Heap memory corruption in GitHub's Markdown table parsing extension

github.com

Post reply on HN
Print view View original

21–30 of 44 posts

Re: Heap memory corruption in GitHub's Markdown table parsing extension

#21

Post by nwmcsween »

staticassertion wrote: post #15
esprehn wrote: post #5

This seems like a good opportunity to use wasm on the server to sandbox the processing of user provided content. Of course they could also try rewriting in a safer language, but given that this already exists and handles all their content, wasm might be a simple defense in depth protection.

What Dropbox did for this sort of thing is ideal. You spawn a child process that has two file handles piped to/from the parent - stdin, stdout. That child process does the scary stuff - parsing. Parsing requires zero system calls. Reading to/from the parent requires only read and write, but not open, so they can only read and write to those file descriptors. And exit. That's it. Seccomp v1 is trivial to apply, gives…

Welcome to 1996 with ucspi

Re: Heap memory corruption in GitHub's Markdown table parsing extension

#22

Post by pornel »

endorphine wrote: post #4
tines wrote: post #3

I am a C++ fanatic---template metaprogramming is a beautiful thing---but I've come to believe that software that handles untrusted user input should never be written in C or C++. It's too difficult to write correct software by hand, memory safe languages are really the only way.

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

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).

Re: Heap memory corruption in GitHub's Markdown table parsing extension

#23

Post by tialaramex »

pjmlp wrote: post #10
bob1029 wrote: post #7

Earlier quoted context omitted.

Does there actually exist any practical way to ensure user input does not cause mischief when authoring C/C++ programs at scale? Are memory-safe languages the only answer?

Yes, the security standards like MISRA and AUTOSAR basically castrate C and C++ into subsets similar to those languages.

Somebody call the Lockpicking Lawyer to shove a paperclip in these "security standards". They're flimsy attempts to excuse still doing something that's a bad idea (programming safety critical software in respectively C and C++) by promising to try harder to achieve the impossible standards needed by humans programming these languages.

And I do mean flimsy. Here's a fun example from a random copy of the AUTOSAR guidelines I found online labelled 17-03. AUTOSAR says if I have two 8-bit signed integers and I add them, that might overflow which is bad. So, what if I simply check that they're both less than 100, no more overflow? "Correct" says the AUTOSAR guide this is apparently OK.

Huh. Signed 8-bit integer. 99 + 99 = -58. This is probably not what the person who purchased your car thought the answer was, I hope whatever accident you just caused isn't fatal.

Re: Heap memory corruption in GitHub's Markdown table parsing extension

#24

Post by pjmlp »

tialaramex wrote: post #23
pjmlp wrote: post #10

Earlier quoted context omitted.

Yes, the security standards like MISRA and AUTOSAR basically castrate C and C++ into subsets similar to those languages.

Somebody call the Lockpicking Lawyer to shove a paperclip in these "security standards". They're flimsy attempts to excuse still doing something that's a bad idea (programming safety critical software in respectively C and C++) by promising to try harder to achieve the impossible standards needed by humans programming these languages. And I do mean flimsy. Here's a fun example from a random copy of the AUTOSAR guidel…

I agree, but our opinion has zero value for whom calls the shots on such industries.

Re: Heap memory corruption in GitHub's Markdown table parsing extension

#25

Post by pjmlp »

nwmcsween wrote: post #21
staticassertion wrote: post #15

Earlier quoted context omitted.

What Dropbox did for this sort of thing is ideal. You spawn a child process that has two file handles piped to/from the parent - stdin, stdout. That child process does the scary stuff - parsing. Parsing requires zero system calls. Reading to/from the parent requires only read and write, but not open, so they can only read and write to those file descriptors. And exit. That's it. Seccomp v1 is trivial to apply, gives…

Welcome to 1996 with ucspi

One thing that I have come to accept is that if one cares about security, the only path is multiple processes, shared library plugins and background threads are a window waiting to be broken.

Re: Heap memory corruption in GitHub's Markdown table parsing extension

#26

Post by olliej »

pjmlp wrote: post #14
esprehn wrote: post #5

This seems like a good opportunity to use wasm on the server to sandbox the processing of user provided content. Of course they could also try rewriting in a safer language, but given that this already exists and handles all their content, wasm might be a simple defense in depth protection.

WASM doesn't protect against heap corruption, because bounds checking doesn't apply inside a linear memory segment.

I think the point was that you can’t corrupt the containing process, and wasm separates code from data (Harvard arch?) so you don’t get arbitrary code exec. Of course if you process output of the wasm in a trusted environment the compromised wasm could generate something that compromises the host, but the same applies to using separate processes and IPC

Re: Heap memory corruption in GitHub's Markdown table parsing extension

#27

Post by olliej »

endorphine wrote: post #4
tines wrote: post #3

I am a C++ fanatic---template metaprogramming is a beautiful thing---but I've come to believe that software that handles untrusted user input should never be written in C or C++. It's too difficult to write correct software by hand, memory safe languages are really the only way.

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

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.

Re: Heap memory corruption in GitHub's Markdown table parsing extension

#28

Post by pjmlp »

olliej wrote: post #26
pjmlp wrote: post #14

Earlier quoted context omitted.

WASM doesn't protect against heap corruption, because bounds checking doesn't apply inside a linear memory segment.

I think the point was that you can’t corrupt the containing process, and wasm separates code from data (Harvard arch?) so you don’t get arbitrary code exec. Of course if you process output of the wasm in a trusted environment the compromised wasm could generate something that compromises the host, but the same applies to using separate processes and IPC

You don't need to compromise the host, or trigger RCE, that is the fallacy of WebAssembly security sales pitch.

It suffices to find a way to corrupt it's internal state and via this attack vector influence its behaviour.

Which yes, boils down to common attacks to separate processes and IPC.

Re: Heap memory corruption in GitHub's Markdown table parsing extension

#29

Post by esprehn »

pjmlp wrote: post #28
olliej wrote: post #26

Earlier quoted context omitted.

I think the point was that you can’t corrupt the containing process, and wasm separates code from data (Harvard arch?) so you don’t get arbitrary code exec. Of course if you process output of the wasm in a trusted environment the compromised wasm could generate something that compromises the host, but the same applies to using separate processes and IPC

You don't need to compromise the host, or trigger RCE, that is the fallacy of WebAssembly security sales pitch. It suffices to find a way to corrupt it's internal state and via this attack vector influence its behaviour. Which yes, boils down to common attacks to separate processes and IPC.

Do you have a POC of such an attack? If true that would mean web browsers would be vulnerable executing wasm because you can intentionally feed it a program with out of bounds access.

Re: Heap memory corruption in GitHub's Markdown table parsing extension

#30

Post by olliej »

pjmlp wrote: post #28
olliej wrote: post #26

Earlier quoted context omitted.

I think the point was that you can’t corrupt the containing process, and wasm separates code from data (Harvard arch?) so you don’t get arbitrary code exec. Of course if you process output of the wasm in a trusted environment the compromised wasm could generate something that compromises the host, but the same applies to using separate processes and IPC

You don't need to compromise the host, or trigger RCE, that is the fallacy of WebAssembly security sales pitch. It suffices to find a way to corrupt it's internal state and via this attack vector influence its behaviour. Which yes, boils down to common attacks to separate processes and IPC.

I don't know of anyone who claims that programs in web assembly are safe internally.

The security claims are entirely that gaining arbitrary execution inside the wasm sandbox does not give you arbitrary execution in the host.

The benefit of a wasm sandbox over a process sandbox is entirely in the overhead reduction - but that does come at the cost of wasm being generally slower than native compilation (oh tradeoffs we will never escape you)

Post reply on HN