search

oasisprotocol / sapphire-paratime

The Sapphire ParaTime monorepo.
https://oasisprotocol.org/sapphire
Apache License 2.0
34 stars 24 forks source link

contracts: fix CBOR unsigned int decoding in Subcall.sol #325

Closed CedarMist closed 3 weeks ago

CedarMist commented 1 month ago

fixes #323

netlify[bot] commented 1 month ago

Deploy Preview for oasisprotocol-sapphire-paratime ready!

Name Link
Latest commit fbca43d0ed7e9d4d3cf23661f43373bdea4e1e79
Latest deploy log https://app.netlify.com/sites/oasisprotocol-sapphire-paratime/deploys/66a88b323494020008a4cb42
Deploy Preview https://deploy-preview-325--oasisprotocol-sapphire-paratime.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

CedarMist commented 1 month ago

Ok, minor last thing.

I need to make the subcall stuff not error out if it encounters and unknown key.

matevz commented 1 month ago

FYI this section https://www.rfc-editor.org/rfc/rfc8949.html#name-tag-validity mentions that the 128- and 256-bit integers are encoded as 0x40 (byte arrays).

Having the _cborParseUInt() split into 64-bit and smaller uints and 128-bit and greater uints codepaths opens potential gas attacks? Should we use gas padding to mitigate this?

CedarMist commented 1 month ago

FYI this section https://www.rfc-editor.org/rfc/rfc8949.html#name-tag-validity mentions that the 128- and 256-bit integers are encoded as 0x40 (byte arrays).

Having the _cborParseUInt() split into 64-bit and smaller uints and 128-bit and greater uints codepaths opens potential gas attacks? Should we use gas padding to mitigate this?

I'd prefer if the caller of the receipt parser does the gas padding if it's deemed necessary rather than building it into the parser itself. But yes, I'll add a note in the code about it.