binast / binjs-ref

Reference implementation for the JavaScript Binary AST format
https://binast.github.io/binjs-ref/binjs/index.html
Other
433 stars 38 forks source link

Stack overflow parsing shift AST of lots of string concatenation #232

Open dominiccooney opened 5 years ago

dominiccooney commented 5 years ago

Here's a 52 KB/700 line file file I encountered in a random sample of httparchive: f724.js.zip

The nature of the file is:

window["fpdefs"] = {
vertex_shader :
'attribute vec3 a_vertexPosition,a_vertexNormal;attribute vec2 a_vert' +
'exTexture;uniform mat4 u_mvMatrix,u_pMatrix,u_nMatrix;uniform vec3 u' +
...
texture :
'/9j/4AAQSkZJRgABAQEAAQABAAD/2wBDABALDA4MChAODQ4SERATGCgaGBYWGDEjJR0o' +
'OjM9PDkzODdASFxOQERXRTc4UG1RV19iZ2hnPk1xeXBkeFxlZ2P/wAALCAIAAgABAREA' +
...

This caused a stack overflow building dictionaries with d477a474500849c9c26abc51de27e7a399f5b77b:

Treating "/.../httparchive/chrome-Nov_1_2017/f724.js" ("")
Parsing.
thread 'large stack dedicated thread' panicked at 'Could not parse source: JsonError(ExceededDepthLimit)', libcore/result.rs:1009:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.
thread 'main' panicked at 'Error in dedicated thread: Any', libcore/result.rs:1009:5

I guess when the Shift AST parser goes away, so will this JSON → AST step. Do you have a pointer to the Rust JavaScript parser you want to replace Shift with?

Yoric commented 5 years ago

As discussed somewhere else, one possible idea is to replace this with ratel.

Yoric commented 5 years ago

See #229 (inactive atm).

Yoric commented 5 years ago

I can confirm that the problem still appears as of tag entropy-0.2.0.

Yoric commented 5 years ago

Cc @RReverser that's a demonstration of the kind of issues we encounter with JSON parsing in real examples already.

RReverser commented 5 years ago

Understood. @dominiccooney Note however that replacing with another parser won't alleviate the problem, it can just increase the limit at which a fatal stack overflow occurs.

dominiccooney commented 5 years ago

@RReverser Yeah I understand that. FWIW I don't think Ratel tip has been able to parse fb_bench for a while. (Maybe I'm holding it wrong.)

Yoric commented 5 years ago

@dominiccooney When you're talking about fb_bench, are you talking about this snapshot or anything more recent?