Add checksum field to bjson#1386
Conversation
| } | ||
| if (bc_get_u32(s, &h)) | ||
| return -1; | ||
| // allow UINT32_MAX as an escape hatch, otherwise updating |
There was a problem hiding this comment.
How are you currently updating them? I also found it tedious 😅
Maybe if we have a Uint8Array which we then encode it might help? Then we could just swap out the version, and properly compute the checksum?
I know you said this is not for adversarial checks, but that seems to be how the bjson module is used lol. Perhaps also some extra language in the docs?
Thinking ahead: do we foresee a world in which we can fully validate the bytecode to we can take any garbage from bjson.read?
There was a problem hiding this comment.
How are you currently updating them? I also found it tedious 😅
With a node one-liner (Buffer.from(s,"base64") -> transform -> buf.toString("base64")), i.e., tediously. :-)
Maybe if we have a Uint8Array which we then encode it might help?
That makes copying base64 output from LLVM's fuzzer more difficult though.
Perhaps also some extra language in the docs?
I'll add that.
do we foresee a world in which we can fully validate the bytecode to we can take any garbage from bjson.read?
No. The fact alone that bjson can contain bytecode (i.e., arbitrary logic) makes that almost intractable.
Helps protect the wire format against data corruption, where corruption should be understood as bit flips, not adversarial input. Fixes: quickjs-ng#1378
|
The standalone failures were due to a bug in standalone.js: it thought the size of the bytecode was > 300,000 bytes when in fact it was only a dozen bytes or so. It only worked because the wire format is somewhat resilient against filler at the end (the reader simply stops reading after the last object) but the checksum definitely caught it. :-) |
Helps protect the wire format against data corruption, where corruption
should be understood as bit flips, not adversarial input.
Fixes: #1378
Possible follow-up work: preset checksum to -1 (meaning: don't check) for built-ins. I don't expect that to make a huge difference though.