Skip to content

decode silently truncates an out-of-range 2-character group instead of rejecting it #7

@7Hugo7

Description

@7Hugo7

Noticed decode accepts some strings it probably shouldn't. A 2-char trailing group should decode to one byte, but the value isn't range-checked before the as u8 cast, so an out-of-range group just gets truncated:

// base45 3.2.0
assert_eq!(base45::decode("ZZ"), Ok(vec![74]));
// "ZZ" -> 35 + 35*45 = 1610, and 1610 as u8 == 74

"ZZ" isn't something encode ever produces, so getting Ok([74]) back means an invalid string decodes fine (and collides with the real encoding of 74).

The 3-char path already guards against this. core_fn returns Err when the value exceeds u16::MAX instead of truncating, but the 2-char branch doesn't have the equivalent check. Mirroring it:

[_first, _second] => {
    let v = d(*_first)? as u32 + d(*_second)? as u32 * SIZE;
    if v > u8::MAX as u32 {
        return Err(DecodeError);
    }
    output.push(v as u8);
}

Valid input is unaffected (in-range 2-char groups decode the same, every byte still round-trips); only the out-of-range cases now error.

Worth noting RFC 9285 only explicitly calls out rejecting the 3-char >65535 case and is quiet on the 2-char one, so it's arguably more of a robustness/consistency thing than a strict spec bug. But since the crate already rejects the 3-char overflow, the 2-char one seemed inconsistent.

Happy to send a PR with the fix + a test if you want.

For what it's worth, I came across this while running Kani (the Rust model checker) over the crate via a tool I'm building, aegisproof.dev. It checked all 256² two-char inputs and flagged this one. Reproduced and reviewed it by hand before posting.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions