Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Mirror] TOB-FUEL-32: ABI decoder is crashing on certain inputs #1353

Closed
digorithm opened this issue Oct 16, 2023 · 3 comments · Fixed by #1426
Closed

[Mirror] TOB-FUEL-32: ABI decoder is crashing on certain inputs #1353

digorithm opened this issue Oct 16, 2023 · 3 comments · Fixed by #1426
Assignees
Labels
bug Issue is a bug

Comments

@digorithm
Copy link
Member

This is a mirror of FuelLabs/fuels-rs#1108.

Opening an issue to investigate these same decoding bugs from the Rust SDK in the Typescript as well.

Trail of Bits (the company doing some audits in our stack) hasn't gone through the TS SDK codebase yet, but some of these problems could also be here; It's worth investigating those proactively.

@digorithm digorithm added bug Issue is a bug package:ABI labels Oct 16, 2023
@arboleya arboleya self-assigned this Oct 17, 2023
@arboleya arboleya removed their assignment Oct 24, 2023
@danielbate danielbate self-assigned this Oct 24, 2023
@arboleya
Copy link
Member

arboleya commented Dec 20, 2023

@danielbate Could this be the same as the 15th item from this list?

@danielbate
Copy link
Member

They are definitely related but I think still should to be handled independently, and an exhaustive type checker would be a further iteration IMO.

All of the issues raised by the TOB investigation were regarding input value rather than input type. This would therefore throw in places where the value for a type was not suitable (overflow / underflow / empty). This can be rectified by adding appropriate validation to our coders to ensure they throw gracefully for these scenarios, rather than either panicking or returning an erroneous value. And adding tests to ensure these new validations have been covered.

An exhaustive type checker as mentioned in #1537 should handle values (over / under etc.), but would be an extension to the above by also checking values under certain type combinations, through complex types. Creating a source of truth for types we do and don't handle, and the values we expect returned for them, and at times the validation errors that they may produce.

We could just go straight to the exhaustive type checker, but that would still raise the need for additional validation that this issue is asking for.

@danielbate danielbate modified the milestones: 1 - Salamander, 2 - Beetle Dec 22, 2023
@danielbate danielbate modified the milestones: 2 - Beetle, 1 - Salamander Jan 4, 2024
@danielbate
Copy link
Member

Resolved in #1426

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants