Stack Exhaustion in WGSL Frontend #5757
Comments
|
@MageWeiG Thank you for filing this. Why isn't the Rust stack sentinel catching this overflow? |
|
How does Naga behave if you feed it the sample without ASAN turned on? Given that Rust enables stack probes by default on all tier 1 platforms, I suspect that ASAN is just catching the problem right before Rust would do so anyway. |
|
Here's what I get: $ unzip crash.zip
Archive: crash.zip
inflating: crash
$ mv crash crash.wgsl
$ naga crash.wgsl
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.09s
Running `/home/jimb/rust/wgpu/target/debug/naga crash.wgsl`
thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Aborted (core dumped)
$ |
|
I'm confident this is not exploitable.
However, it is a bug that Naga blows its stack: it should return an error. It's just not a vulnerability. |
This is correct. The vulnerability is at worst a DoS. EDIT: (this is only the case with standalone Rust executables, when linking this into C code the story is different) |
|
This issue is still open as a critical security vulnerability with a rating of 9.8 out of 10 as per https://nvd.nist.gov/vuln/detail/CVE-2024-36761. This prevents using anything wgpu based in a corporate context where libraries are scanned against open vulnerabilities on nvd.nist.gov. Is there a way you can either correct the CVSS (Version 3) rating of this CVE or fix issue #5759 please? As we stand today, this issue prevents usage of Rust for UI development in large corporations which do rely on nvd.nist.gov. Thanks! |
|
I have no idea how we would even do this to change the vulnerability on nist. This would only affect untrusted shaders. |
|
Regarding the vulnerability rating, I did some digging on the nist nvd side and found the following:
I am more than happy to submit the request, but to do so I need to better understand the nature of the exposure, in particular the "Attack Vector", "Attack Complexity" and "Privileges Required" dimensions. Currently the vulnerability is rated as AV: Network (i.e. remote executable), AC: Low (quote from CVSS Calculator "Specialized access conditions or extenuating circumstances do not exist. An attacker can expect repeatable success against the vulnerable component.") and PR: None. Is this really an accurate assessment of the situation? This would mean that anybody with a network connection to a system running naga, without any privileges on that system would be able to exploit the vulnerability with repeatable results. From what I understand from the comments in this issue, that does not seem to be the case? |
|
The severity doesn't make sense. I'll take care of requesting a rescore. |
|
@dangrazh From playing with the calculator, it seems like the "confidentiality" and "integrity" impact of this bug is minimal. Changing those to "None" lowers the rating to 7.5. I'm not sure that is going to reassure anyone. This is not a hard bug to fix, it's just dumb. So maybe the best course is just to go ahead and fix it, even though it's not actually putting anyone at risk. |
|
@jimblandy I would agree, if it is a relatively simple bug to fix, fixing the bug and closing the CVE based on that is certainly the best course of action. Thanks for getting back on this one! |
|
For the record, I've filed a request with the National Vulnerability Database to have this reclassified:
Although it's true that simply running Naga does not open one up to network attacks, I didn't challenge the "Network" attack vector because, in the case of Firefox, we do actually pass untrusted data from the network to Naga. This would also be true of someone running some sort of shader playground or some web app that let people write their own shaders. |
I encourage the term "stack exhaustion" because the other term is commonly misinterpreted as "stack BUFFER overflow" -- a different kettle of fish entirely.
On Windows it would result in EXCEPTION_STACK_OVERFLOW and is not exploitable. In rare cases stack exhaustion it might be exploitable if the program (or even Windows APIs, in some cases) catches the exception and mishandles it: https://www.blackhat.com/presentations/bh-usa-07/Dowd_McDonald_and_Mehta/Whitepaper/bh-usa-07-dowd_mcdonald_and_mehta.pdf |
|
@jimblandy I think it's even lower than 7.5.
This brings it down to 4.3: https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:N/AC:L/PR:N/UI:R/S:U/C:N/I:N/A:L&version=3.1 Still seems too high to me, but CVSS is structured for sites and services and isn't great for client programs. |
|
Okay, I missed the "user interaction" bit. Thanks for the clarification about "availability". |
cwfitzgerald
changed the title
cwfitzgerald
added
type: bug
That raises a fair point. It's possible for the severity of a CVE in a library to be different from the severity of that flaw when embedded in an application that uses it incorrectly, unsafely, or even just differently than expected. Newer versions of CVSS try to represent that by having an additional "Environmental" score. Arguably wgpu was created primarily to implement the WebGPU API, and that API was specified assuming untrustworthy web input. From that POV discounting the Network attack vector in the library itself would be unexpected. That will, however, overstate the severity of bugs from the POV of self-contained applications. They should calculate their own modified environmental CVSS score. For example, setting the Modified Attack Vector to Physical ("physical" access required to hack the compiled program?) gives you an environmental score of 2.1 (Low)—which is still too high for that situation IMHO! In a browser, setting the "Availability Requirement" to "Low" (a browser doesn't have to be a DDOS resistant server) results in an environmental score of 3.6 (Low) compared to the base score of 4.3 (Medium) Or you could decide the library is primarily for self-contained graphics applications and rate flaws accordingly, and downstream browsers would have to report their advisories with an up-rated Environmental score representing the fact that now the Network is an attack vector. Of course I think you should take the browser-centric POV because showing the high end of the range will make it less likely people will ignore announcements without a second look, and sheer numbers of affected users. But whichever approach you take, you should document that operating perspective in your future SECURITY.md policy |
It's currently trivial to write a shader that causes the wgsl parser to recurse too deeply and overflow the stack. This patch makes the parser return an error when recursing too deeply, before the stack overflows. It makes use of a new function Parser::track_recursion(). This increments a counter returning an error if the value is too high, before calling the user-provided function and returning its return value after decrementing the counter again. Any recursively-called functions can simply be modified to call track_recursion(), providing their previous contents in a closure as the argument. All instances of recursion during parsing call through either Parser::statement(), Parser::unary_expression(), or Parser::type_decl(), so only these functions have been updated as described in order to keep the patch as unobtrusive as possible. A value of 256 has been chosen as the recursion limit, but can be later tweaked if required. This avoids the stack overflow in the testcase attached to issue gfx-rs#5757.
[edit by @jimblandy: This has been reported as CVE-2024-36761.]
When I was using oss-fuzz to test the fuzzer wgsl_parser in my project, I found a stack overflow vulnerability that could trigger a crash by typing enough '(' into the fuzzer.
The crash information is as follows:
Attached is the test sample.
crash.zip
The text was updated successfully, but these errors were encountered: