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

Trailing characters on zero values are accepted #389

Open
Rjected opened this issue Jun 27, 2024 · 2 comments
Open

Trailing characters on zero values are accepted #389

Rjected opened this issue Jun 27, 2024 · 2 comments

Comments

@Rjected
Copy link
Contributor

Rjected commented Jun 27, 2024

Version

1.12.3

Platform

Darwin Dans-MacBook-Pro-4.local 23.3.0 Darwin Kernel Version 23.3.0: Wed Dec 20 21:30:44 PST 2023; root:xnu-10002.81.5~7/RELEASE_ARM64_T6000 arm64

Description
I was noticing that reth was succeeding when it got eth_getStorageAt calls that had a 67-character "zero" slot:
0x00000000000000000000000000000000000000000000000000000000000000000

I wrote a crude test to check this:

    #[test]
    fn test_serde_invalid_size_error_zero() {
        // Test that if we add a character to a zero value for the given number of bits, we get an
        // error.
        const_for!(BITS in SIZES {
            const LIMBS: usize = nlimbs(BITS);

            // repeated pattern of 0
            let mut zero_bytes = BITS / 8;
            if BITS % 8 != 0 {
                zero_bytes += 1;
            }

            let mut serialized = String::from("\"0x");

            // push zero bytes
            for _ in 0..zero_bytes {
                serialized.push('0');
                serialized.push('0');
            }
            // add one zero
            serialized.push('0');

            // edge case: zero bits, we should only be able to deser `0x0` for the zero bit Uint
            if BITS == 0 {
                serialized.push('0');
            }

            serialized.push('\"');

            // ensure format of serialized value is correct ("0x...")
            assert_eq!(&serialized[..3], "\"0x");
            // last character should be a quote
            assert_eq!(&serialized[serialized.len() - 1..], "\"");

            let deserialized = serde_json::from_str::<Uint<BITS, LIMBS>>(&serialized);
            assert!(deserialized.is_err(), "{BITS} {serialized}");
        });
    }

To be compliant with QUANTITY, these should be rejected.

@Rjected
Copy link
Contributor Author

Rjected commented Jun 27, 2024

Realizing we had this bug before, including a test, and I had fixed it:
#229

@Rjected
Copy link
Contributor Author

Rjected commented Jun 27, 2024

I guess this boils down to:
If we want to allow permissive U256 deser for other formats, we need to use a separate type or helper method for deserializing U256 in rpc

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

No branches or pull requests

1 participant