mirror of
https://github.com/pezkuwichain/revive.git
synced 2026-10-20 18:56:17 +00:00
Fix/ny unaligned mload native byteorder (#562)
Another bug found by @jubnzv: `--newyork`: an unaligned full-word `mload`/`mstore` overlapping a word kept native (little-endian) by the heap optimization returned byte-swapped bytes. --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
9b177f6400
commit
c18d29b861
5 files changed
+83
-4
No files matched your search
@@ -8,6 +8,7 @@ Supported `polkadot-sdk` rev: `2604.2.0`
|
||||
|
||||
- Restored the top-level `revive_version` field in `--standard-json` output and the `resolc_version` field in `--combined-json` output, both of which stopped being populated in `v0.4.0`. [#557](https://github.com/paritytech/revive/pull/557)
|
||||
- ICE triggered by `--disable-solc-optimizer --newyork where inlined loops could emit an unterminated basic block in the inlined entrypoint. [#561](https://github.com/paritytech/revive/pull/561)
|
||||
- `--newyork`: an unaligned full-word `mload`/`mstore` overlapping a word kept native (little-endian) by the heap optimization returned byte-swapped bytes.
|
||||
|
||||
## v1.3.0
|
||||
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
pragma solidity ^0.8.28;
|
||||
|
||||
/// Soundness PoC (newyork heap optimization): a full-word `mload` at a
|
||||
/// non-word-aligned offset that overlaps a word the heap analysis classified
|
||||
/// as native (little-endian, byte-swap elided) reads that word's bytes in the
|
||||
/// wrong order.
|
||||
///
|
||||
/// `mstore(0x20, PAT)` is word-aligned and does not escape, so newyork keeps
|
||||
/// it little-endian (native mode, no byte-swap). `mload(0x08)` reads
|
||||
/// `[0x08, 0x28)`, overlapping the native word at `0x20`, but is itself
|
||||
/// unaligned so it lowers to a big-endian (byte-swapped) read. The two
|
||||
/// accesses disagree on byte order for the shared bytes, so the load returns
|
||||
/// byte-swapped garbage. The fix taints every word an unaligned full-word
|
||||
/// access covers, forcing those words big-endian for all accesses.
|
||||
contract UnalignedMload {
|
||||
function f() external pure returns (bytes32 r) {
|
||||
assembly {
|
||||
mstore(0x20, 0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20)
|
||||
r := mload(0x08)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -317,6 +317,18 @@ case!(
|
||||
unaligned_mstore_bug,
|
||||
);
|
||||
|
||||
sol!(
|
||||
contract UnalignedMload {
|
||||
function f() external pure returns (bytes32);
|
||||
}
|
||||
);
|
||||
case!(
|
||||
"UnalignedMloadNativeBug.sol",
|
||||
UnalignedMload,
|
||||
fCall,
|
||||
unaligned_mload_native_bug,
|
||||
);
|
||||
|
||||
sol!(
|
||||
contract ConstReturnOverflowBug {
|
||||
function bug() external pure returns (uint256);
|
||||
|
||||
@@ -469,6 +469,29 @@ fn unaligned_mstore_forwarding() {
|
||||
run_differential(actions);
|
||||
}
|
||||
|
||||
/// Regression (newyork heap optimization): a full-word `mload` at a
|
||||
/// non-word-aligned offset that overlaps a word classified native
|
||||
/// (little-endian, byte-swap elided) returned byte-swapped garbage. In the
|
||||
/// PoC `mstore(0x20, PAT)` is word-aligned and stays native, but `mload(0x08)`
|
||||
/// reads `[0x08, 0x28)` — overlapping the native word `0x20` — and lowers to a
|
||||
/// big-endian read, so the two accesses disagree on byte order for the shared
|
||||
/// bytes. The heap analysis now taints every word an unaligned full-word
|
||||
/// access covers, forcing them big-endian so all accesses agree. Compared
|
||||
/// newyork-PVM vs solc-EVM.
|
||||
#[test]
|
||||
fn unaligned_mload_native_byte_order() {
|
||||
let mut actions = instantiate("contracts/UnalignedMloadNativeBug.sol", "UnalignedMload");
|
||||
actions.push(Call {
|
||||
origin: TestAddress::Alice,
|
||||
dest: TestAddress::Instantiated(0),
|
||||
value: 0,
|
||||
gas_limit: None,
|
||||
storage_deposit_limit: None,
|
||||
data: Contract::unaligned_mload_native_bug().calldata,
|
||||
});
|
||||
run_differential(actions);
|
||||
}
|
||||
|
||||
/// Regression: `to_llvm.rs::get_or_create_return_block` (and revert / Stop
|
||||
/// siblings) build the constant offset/length via
|
||||
/// `context.xlen_type().const_int(const_offset, false)`, which silently
|
||||
|
||||
@@ -268,10 +268,10 @@ impl HeapAnalysis {
|
||||
} else {
|
||||
self.has_dynamic_accesses = true;
|
||||
}
|
||||
if *region == MemoryRegion::Unknown && !pattern.is_aligned() {
|
||||
if !pattern.is_aligned() {
|
||||
if let Some(address) = static_offset {
|
||||
if address % BYTE_LENGTH_WORD as u64 != 0 {
|
||||
self.tainted_regions.insert(word_align(address));
|
||||
self.taint_unaligned_access(address);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -585,6 +585,21 @@ impl HeapAnalysis {
|
||||
}
|
||||
}
|
||||
|
||||
/// Taints every word a static, non-word-aligned full-word access (`mload`/`mstore`) covers.
|
||||
///
|
||||
/// EVM memory is big-endian; the native-mode optimization keeps a word's bytes little-endian
|
||||
/// (skipping the byte-swap) only when *every* access to that word is word-aligned. An unaligned
|
||||
/// full-word access at `address` reads or writes the raw bytes of `[address, address + 32)`,
|
||||
/// spanning the two words `word_align(address)` and `word_align(address) + 32`. If either word
|
||||
/// were left a native candidate, an aligned native (little-endian) access to it and this
|
||||
/// unaligned (big-endian) access would disagree on byte order for the overlapping bytes,
|
||||
/// byte-swapping the result. Tainting both covered words forces them big-endian so all accesses
|
||||
/// agree. `mstore(0x20, w); r := mload(0x08)` is the canonical trigger: the aligned store is
|
||||
/// native but the unaligned load reads word `0x20` in the wrong order without this taint.
|
||||
fn taint_unaligned_access(&mut self, address: u64) {
|
||||
self.taint_range(Some(address), Some(BYTE_LENGTH_WORD as u64));
|
||||
}
|
||||
|
||||
/// Taints all word-aligned memory regions in a range.
|
||||
/// If the range is too large, treats it as a dynamic access instead.
|
||||
fn taint_range(&mut self, start: Option<u64>, len: Option<u64>) {
|
||||
@@ -783,8 +798,12 @@ impl HeapAnalysis {
|
||||
Expression::MLoad { offset, .. } => {
|
||||
let _ = self.classify_access(offset);
|
||||
self.track_variable_access(offset);
|
||||
if self.extract_static_offset(offset).is_none() {
|
||||
self.has_dynamic_accesses = true;
|
||||
match self.extract_static_offset(offset) {
|
||||
None => self.has_dynamic_accesses = true,
|
||||
Some(address) if address % BYTE_LENGTH_WORD as u64 != 0 => {
|
||||
self.taint_unaligned_access(address);
|
||||
}
|
||||
Some(_) => {}
|
||||
}
|
||||
}
|
||||
Expression::Keccak256 { offset, length } => {
|
||||
|
||||
Reference in new issue
Block a user