Json parse keys limit

[afwn90cj93201nixr2e1re]

owasp-modsecurity/ModSecurity#2060

should we do same here?
coz native Json.parse seems also vulnerable.
i mean i can recompile v8/nodejs, add additional params, like keyLimits, to prevent it, but, that's not a good solution.

also im using express with body parser, which also can be vulnerable in this case, also with limit: 1mb, as you can see.

i also using ajv for object validating, but it's can validate it only after body parser parse it to object, so...

[afwn90cj93201nixr2e1re]

@gireeshpunathil ping.

[gireeshpunathil]

acknowledged, will review and get back.

[afwn90cj93201nixr2e1re]

ok, waiting for it.

[gireeshpunathil]

ok, I am not sure this need to be catered to by yieldable-json. The tool provides the infrastructure capability for processing JSON, and depending on the workload / use case, the middleware or app should take the decision to limit the payload volume / key-value count etc. For example, we do provide the intensity value that tells when to pause processing and return to the event loop. A caller in the upper stack could easily leverage this and implement the rate-limiter for his payload.

Having said that, I am open for arguments!

[afwn90cj93201nixr2e1re]

I mean, we also can't check how much keys here before we not parse it to object, then we can get it from Object(obj).keys.length, but parse still done, and it can takes 10+ min fpr each malicious payload.

Can you show simple example with intensity + express app?

[afwn90cj93201nixr2e1re]

https://github.com/ibmruntimes/yieldable-json/blob/master/test/test-parse-async.js#L6

for example, how i can parse only name and age and skip gender with intensity?

[gireeshpunathil]

something like this:

const yj = require('yieldable-json')
var flag = false
monitorMaliciousPayload() {
  setImmediate(() => {
    if (!done)       // parsing did not complete in 8192 iterations, malicious one, abort it!
      abortRequest()
  })
}
yj.parseAsync(str, {intensity: 8192} (e, obj) => {
  done = true
  // all well, do with the object
})
monitorMaliciousPayload()

[afwn90cj93201nixr2e1re]

so, i fixed it inside body parser, now i just provide limit, inside parse function inside json.js, then added second arg to JSON.parse function, and created local variable, which store keys length on each func call, so, if limit reached i just throw an error, and seems like JSON.parse stopped working, but im not sure, is v8 really stops parse json on custom throw Error('message') inside reviver function.

Can you check it?

https://chromium.googlesource.com/v8/v8/+/refs/heads/master/src/json/json-parser.cc#173

is that gonna stop parsing on throw error or i should return undefined to stop working?