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

Resolve memory overflow when b256::TryFrom<Bytes> is not 32 bytes #6136

Merged
merged 7 commits into from
Jun 19, 2024

Conversation

bitzoic
Copy link
Member

@bitzoic bitzoic commented Jun 17, 2024

Description

Currently, when calling the TryFrom implementation for b256 from Bytes, if the Bytes has less than 32 bytes a memory overflow error will occur. If there is less than 32 bytes, None is now returned.

Example:

    let mut bytes = Bytes::with_capacity(32);
    let mut i = 0;
    while i < 32 {
        // 0x33 is 51 in decimal
        bytes.push(51u8);
        i += 1;
    }
    let res = b256::try_from(bytes);
    assert(res.unrwap() == 0x3333333333333333333333333333333333333333333333333333333333333333);

Closes #6129

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@bitzoic bitzoic added bug Something isn't working lib: std Standard library labels Jun 17, 2024
@bitzoic bitzoic self-assigned this Jun 17, 2024
@bitzoic bitzoic requested review from a team as code owners June 17, 2024 03:58
Copy link
Contributor

@IGI-111 IGI-111 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think left padding is the correct behavior. Converting less than 32 bytes to a b256 should fail.

You may argue this for u256 though I still think it's dubious to assume user intent there. But for b256 it's categorically erroneous.

@IGI-111
Copy link
Contributor

IGI-111 commented Jun 18, 2024

LGTM but can you update the title and PR message to reflect the changes?

@K1-R1 K1-R1 requested a review from a team June 18, 2024 10:41
@bitzoic bitzoic changed the title Resolve memory overflow when b256::TryFrom<Bytes> has less than 32 bytes Resolve memory overflow when b256::TryFrom<Bytes> is not 32 bytes Jun 19, 2024
@IGI-111 IGI-111 merged commit 2cd6a59 into master Jun 19, 2024
37 checks passed
@IGI-111 IGI-111 deleted the bitzoic-6129 branch June 19, 2024 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lib: std Standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

b256::try_from should check against length is not 32 rather than greater than
4 participants