Skip to content

Conversation

Copy link

Copilot AI commented Nov 27, 2025

Option<T>::from_ssz_bytes accepted trailing data when deserializing the None variant (selector 0x00), violating the SSZ Union spec which requires zero-length payload for None.

// Before: silently returns Ok(None), ignoring trailing 0xFF
let dirty = &[0x00, 0xFF];
<Option<u8>>::from_ssz_bytes(dirty) // Ok(None) ❌

// After: rejects trailing bytes
<Option<u8>>::from_ssz_bytes(dirty) // Err(InvalidByteLength { len: 1, expected: 0 }) ✓

Changes:

  • Validate body.is_empty() when selector is 0x00, return InvalidByteLength error otherwise
  • Add test covering rejection of trailing bytes and valid encodings
Original prompt

This section details on the original issue you should resolve

<issue_title>SSZ Union Deserialization Vulnerable to Trailing Data for Certain Variants</issue_title>
<issue_description>## Bug Summary

When the None variant (selector 0x00) is selected, the payload should be strictly zero-length. Vulnerable implementations may parse the selector, then ignore any trailing bytes in the payload, leading to hash mismatches.

Details

The from_ssz_bytes method for Option<T> in ssz/src/decode/impls.rs acts as a Union. When the 0u8 selector (representing None) is encountered, the body (which may contain trailing dirty bytes) is ignored:

impl<T: Decode> Decode for Option<T> {
    // ...
    fn from_ssz_bytes(bytes: &[u8]) -> Result<Self, DecodeError> {
        let (selector, body) = split_union_bytes(bytes)?; // consumes selector and body
        match selector.into() {
            0u8 => Ok(None), // Returns Ok(None) immediately, ignoring `body`
            1u8 => <T as Decode>::from_ssz_bytes(body).map(Option::Some),
            other => Err(DecodeError::UnionSelectorInvalid(other)),
        }
    }
}

PoC

use ssz::{Decode}; // Decode trait is needed for Option<u8>::from_ssz_bytes
use smallvec::{smallvec, SmallVec};

fn main() {
    println!("--- Lighthouse (ssz) PoC: Dirty Tail on None ---");

    // 1. Construct a dirty input
    // Selector 0x00 for None variant, followed by a dirty byte 0xFF
    let dirty_bytes: SmallVec<[u8; 2]> = smallvec![0x00, 0xFF];

    println!("Attempting to deserialize dirty bytes: {:x?}", dirty_bytes);

    // 2. Attempt to deserialize Option<u8> directly
    // In ethereum_ssz, Option<T> is treated as a Union type where 0x00 is None.
    let result = <Option<u8>>::from_ssz_bytes(&dirty_bytes);

    match result {
        Ok(deserialized_value) => {
            // If deserialization was successful, the vulnerability is confirmed.
            if deserialized_value.is_none() {
                println!("SUCCESS: Deserialized to None variant as expected (dirty tail ignored).");
                println!("VULNERABILITY CONFIRMED: `ssz` accepts dirty tail on Option<u8> (None variant).");
                println!("Deserialized value: {:?}", deserialized_value);
            } else {
                eprintln!("FAILURE: Deserialization returned Some, which is unexpected for selector 0x00.");
                eprintln!("Deserialized value: {:?}", deserialized_value);
            }
        },
        Err(e) => {
            // If an error is thrown, it means the client correctly rejected the dirty input.
            println!("Client correctly rejected dirty bytes. Not vulnerable. Error: {:?}", e);
        }
    }

    println!("--- PoC End ---");
}

Output:

--- Lighthouse (ssz) PoC: Dirty Tail on None ---
Attempting to deserialize dirty bytes: [0, ff]
SUCCESS: Deserialized to None variant as expected (dirty tail ignored).
VULNERABILITY CONFIRMED: `ssz` accepts dirty tail on Option<u8> (None variant).
Deserialized value: None
--- PoC End ---

Fix

The from_ssz_bytes method for Option<T> should be updated to ensure that if the 0u8 selector is chosen, the body is empty. If body contains any bytes, it should return an error.</issue_description>

<agent_instructions>Please confirm if this vulnerability exists by adding a test to trigger it, and then raising a PR to fix it if the issue does indeed exist.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@michaelsproul Nice find, thank you!

Thankfully I don't think this is exploitable in Lighthouse (else you could have claimed a bug bounty), but we will fix it.</comment_new>


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Fix SSZ union deserialization vulnerability for None variant Fix SSZ Union deserialization accepting trailing bytes for None variant Nov 27, 2025
Copilot AI requested a review from michaelsproul November 27, 2025 06:01
Copilot finished work on behalf of michaelsproul November 27, 2025 06:01
@macladson macladson marked this pull request as ready for review November 27, 2025 12:20
@codecov
Copy link

codecov bot commented Nov 27, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.39%. Comparing base (ead9358) to head (b3be070).

Files with missing lines Patch % Lines
ssz/src/decode/impls.rs 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #65      +/-   ##
==========================================
- Coverage   76.62%   76.39%   -0.23%     
==========================================
  Files          14       14              
  Lines         800      805       +5     
==========================================
+ Hits          613      615       +2     
- Misses        187      190       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Successfully merging this pull request may close these issues.

SSZ Union Deserialization Vulnerable to Trailing Data for Certain Variants

2 participants