recmo / uint

Rust Uint crate using const-generics
MIT License
169 stars 39 forks source link

Trailing characters on zero values are accepted #389

Open Rjected opened 2 months ago

Rjected commented 2 months ago

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 commented 2 months ago

Realizing we had this bug before, including a test, and I had fixed it: https://github.com/recmo/uint/pull/229

Rjected commented 2 months ago

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