Skip to content

fix: replace transmute UB in error constructors with safe alternatives#58

Merged
nlordell merged 3 commits into
nlordell:mainfrom
LuciferYang:fix-transmute-ub
Apr 20, 2026
Merged

fix: replace transmute UB in error constructors with safe alternatives#58
nlordell merged 3 commits into
nlordell:mainfrom
LuciferYang:fix-transmute-ub

Conversation

@LuciferYang

@LuciferYang LuciferYang commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Summary

pie() used unsafe { mem::transmute(kind) } to create a ParseIntError from an IntErrorKind, and tfie() used unsafe { mem::transmute(()) } to create a TryFromIntError. Both rely on unstable internal layouts of standard library types.

Starting with Rust nightly nightly-2026-04-18 (commit e9e32aca5, 2026-04-17) on Linux x86_64, TryFromIntError grew from 0 to 1 byte, causing tfie() to fail at compile time:

error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
  --> src/error.rs:16:14
   |
16 |     unsafe { mem::transmute(()) }
   |              ^^^^^^^^^^^^^^
   |
   = note: source type: `()` (0 bits)
   = note: target type: `TryFromIntError` (8 bits)

pie() still compiles today but is equally fragile — it depends on IntErrorKind and ParseIntError having the same layout.

Changes

Replace both functions with safe stdlib operations:

  • pie(kind): unsafe { mem::transmute(kind) } → match on IntErrorKind and trigger real parse errors via from_str_radix. Stays const fn.
  • tfie(): unsafe { mem::transmute(()) }u8::try_from(-1i8).unwrap_err(). No longer const fn since TryFrom isn't const-stable yet, but no call site uses it in const context.

Other changes per review feedback:

  • Use unreachable!() for the wildcard match arm (internal-only input)
  • Remove IntErrorKind::Zero from pie() (not used by the library)

Verification

  • All 207 existing tests pass on stable, nightly (macOS), and nightly (Linux x86_64 via Docker)
  • Confirmed the original code fails to compile on nightly-2026-04-18 Linux x86_64
  • Confirmed the fix compiles and passes on nightly-2026-04-18 Linux x86_64
  • All CI lint checks pass: cargo fmt, cargo clippy (workspace, serde, macros, llvm-intrinsics)

`tfie()` used `mem::transmute(())` to create a `TryFromIntError`, and
`pie()` used `mem::transmute(kind)` to create a `ParseIntError`.  Both
rely on unstable internal layouts of standard library types.

Starting with Rust nightly > 2026-04-16 on Linux x86_64,
`TryFromIntError` grew from 0 to 1 byte, making the `tfie()` transmute
a compilation error (size mismatch).  `pie()` still compiles today but
is equally fragile.

Replace both with safe stdlib operations:
- `tfie()`: `u8::try_from(-1i8).unwrap_err()`
- `pie()`: match on `IntErrorKind` and trigger real parse errors

The functions are no longer `const fn` because the safe alternatives
call non-const methods.  No existing call site uses them in const
context, so this is a compatible change.

All 207 existing tests continue to pass on both stable and nightly.
Comment thread src/error.rs Outdated
IntErrorKind::PosOverflow => u8_parse_error("256"),
IntErrorKind::NegOverflow => i8_parse_error("-129"),
IntErrorKind::Zero => zero_parse_error(),
_ => u8_parse_error("?"), // fallback for future variants

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This should be unreachable!() IMO - the input kind is limited to the uses of this function internal to the library, so we know that this branch will never be reached.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, switched to unreachable\!().

Comment thread src/error.rs Outdated
Comment on lines +18 to +20
fn u8_parse_error(s: &str) -> ParseIntError {
s.parse::<u8>().unwrap_err()
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

If we use https://doc.rust-lang.org/stable/std/primitive.u8.html#method.from_str_radix, then the function can stay const:

const fn u8_parse_error(s: &str) -> std::num::ParseIntError {
    let Err(err) = u8::from_str_radix(s, 10) else {
        panic!("not a parse error!");
    };
    err
}

fn main() {
    let x = u8_parse_error("-1");
    println!("{x:?}!");
}

Same for the i8_parse_error function, (allowing pie to stay const).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nice, from_str_radix works perfectly here. pie() and the helpers are const fn again. tfie() stays non-const since TryFrom is not const-stable yet, but no call site needs it in const context anyway.

Comment thread src/error.rs Outdated
IntErrorKind::InvalidDigit => u8_parse_error("?"),
IntErrorKind::PosOverflow => u8_parse_error("256"),
IntErrorKind::NegOverflow => i8_parse_error("-129"),
IntErrorKind::Zero => zero_parse_error(),

@nlordell nlordell Apr 19, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The Zero is only used in the test below, so it can be removed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed. Also dropped the NonZeroU32 import that was only there for the Zero test.

@nlordell nlordell left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Generally agree with the change, thanks very much!

Left some small comments that I want to be resolved before merging. I can cut a 1.5.3 release afterwards as well.

- Use u8::from_str_radix / i8::from_str_radix to keep pie() and helpers const
- Replace wildcard fallback with unreachable!() since kind is internal
- Remove IntErrorKind::Zero from pie() match (only used in tests)
- Move Zero variant test to a separate test function
The Zero variant was removed from pie() match. The test was verifying
stdlib's NonZeroU32 parsing, not any library functionality.
@nlordell nlordell merged commit 87e3457 into nlordell:main Apr 20, 2026
1 check passed
@LuciferYang

Copy link
Copy Markdown
Contributor Author

Thank you @nlordell ~

@nlordell

Copy link
Copy Markdown
Owner

Thanks again for the contribution! Released: https://crates.io/crates/ethnum/1.5.3

@LuciferYang

Copy link
Copy Markdown
Contributor Author

Thank you again @nlordell

Xuanwo pushed a commit to lance-format/lance that referenced this pull request May 7, 2026
## Summary

Upgrade transitive dependency `ethnum` from 1.5.2 to 1.5.3 and revert
the nightly toolchain pin from #6570.

Dependency chain: `lance-arrow` → `jsonb 0.5.6` → `ethnum 1.5.3`

## Root Cause

`ethnum 1.5.2` used `unsafe { mem::transmute(()) }` to construct
`TryFromIntError`, which broke on nightly > 2026-04-16 when
`TryFromIntError` grew from 0 to 1 byte on Linux x86_64.

`ethnum 1.5.3`
([nlordell/ethnum-rs#58](nlordell/ethnum-rs#58))
replaced all transmute with safe stdlib operations, so the pin is no
longer needed.

## Test plan

- [x] Verified ethnum 1.5.3 compiles on `nightly-2026-04-18` (the
version that broke 1.5.2) in Docker Linux x86_64
- [x] `linux-build` job should pass with unpinned nightly
pull Bot pushed a commit to Bloxxix/stellar-core that referenced this pull request May 22, 2026
…lar#5287)

The causal chain here is:

- oss-fuzz wants to fuzz with sanitizers turned on (a generally good
idea)
- rust sanitizer builds have to be both unified builds and _nightly_
builds (the sanitizer flag is rust-nightly-only)
  - our rust build depends on `ethnum`, specifically version `1.5.0`
- `ethnum 1.5.0` has a bug in it that our pinned rust `1.95.0` doesn't
mind, but rust nightly rejects; it's
[fixed](nlordell/ethnum-rs#58) in `1.5.3`
- `1.5.3` otherwise seems to work fine and the release notes show only
some optimizations and bugfixes.
- There's some _theoretical_ potential for divergence (eg. where
observable bugs in `ethnum` were fixed:
nlordell/ethnum-rs#44) but .. I don't see any
way for this to actually get hit in practice: this PR is a bump to the
`ethnum` dependency _outside_ any of the soroban production-build deps
(they all have their own pinned ones in production builds) and that
dependency only exists, as far as I can tell, because `stellar-xdr`
links in `ethnum` to be able to do u256/i256 ops, which .. we don't
actually use anywhere in the non-soroban-host rust code (eg. the bridge
helpers). There's no use of `i256`, `u256`, `I256`, `U256`, or `ethnum`
in any rust code in our tree outside the soroban crates. So I don't
_think_ this can cause any divergence in production. But, you know, one
can always miss something!

If that very narrow possible I-don't-see-how risk is still too much to
swallow we can wait for a protocol boundary, but this will I think
basically block oss-fuzz integration until it's resolved.

(There's another option which is to turn off the sanitizers, I guess; I
am going to try that in the meantime to see if I can get it working, but
I'm not sure how likely that is nor whether it's a particularly good
idea longer-term.)
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.

2 participants