Skip to content

Commit a1a4f8e

Browse files
authored
Clip OOB values in call data load and copy (#448)
The OOB logic is implemented in the runtime but truncation circumvented it. Closes #263 --------- Signed-off-by: xermicus <cyrill@parity.io> Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>
1 parent c1ce984 commit a1a4f8e

File tree

5 files changed

+68
-12
lines changed

5 files changed

+68
-12
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ This is a development pre-release.
66

77
Supported `polkadot-sdk` rev: `unstable2507`
88

9+
### Fixed
10+
- OOB access in `calldataload` and `calldatacopy` should always produce zero values instead of consuming all gas.
11+
912
## v0.6.0
1013

1114
This is a development pre-release.

crates/integration/codesize.json

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"Baseline": 911,
3-
"Computation": 2337,
4-
"DivisionArithmetics": 14488,
5-
"ERC20": 17189,
6-
"Events": 1672,
7-
"FibonacciIterative": 1454,
8-
"Flipper": 2106,
9-
"SHA1": 7814
2+
"Baseline": 864,
3+
"Computation": 2323,
4+
"DivisionArithmetics": 14475,
5+
"ERC20": 17229,
6+
"Events": 1663,
7+
"FibonacciIterative": 1421,
8+
"Flipper": 2154,
9+
"SHA1": 7751
1010
}

crates/integration/contracts/MemoryBounds.sol

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,43 @@ pragma solidity ^0.8.24;
2121
"Instantiated": 0
2222
}
2323
}
24+
},
25+
{
26+
"Call": {
27+
"dest": {
28+
"Instantiated": 0
29+
},
30+
"data": "489eb33e00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001a300000000000000000000000000000000000000000000000000000000000000"
31+
}
32+
},
33+
{
34+
"Call": {
35+
"dest": {
36+
"Instantiated": 0
37+
},
38+
"data": "db1ff7bb00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001a300000000000000000000000000000000000000000000000000000000000000"
39+
}
2440
}
2541
]
2642
}
2743
*/
2844

2945
contract MemoryBounds {
46+
function oobLoad(bytes memory data) public pure returns (uint256, bytes memory) {
47+
uint256 result1;
48+
assembly {
49+
let ptr := mload(add(data, 0x20))
50+
result1 := calldataload(ptr)
51+
}
52+
return (result1, data);
53+
}
54+
55+
function oobCopy(bytes memory data) public pure returns (uint256, bytes memory) {
56+
assembly {
57+
calldatacopy(0, 0x3a0000000000000000000000000000000000, 0x20)
58+
}
59+
}
60+
3061
fallback() external {
3162
assembly {
3263
// Accessing OOB offsets should always work when the length is 0.

crates/llvm-context/src/polkavm/context/mod.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,28 @@ impl<'ctx> Context<'ctx> {
10541054
.into_int_value())
10551055
}
10561056

1057+
/// Clip a memory offset to the maximum value that fits into a register.
1058+
pub fn clip_to_xlen(
1059+
&self,
1060+
value: inkwell::values::IntValue<'ctx>,
1061+
) -> anyhow::Result<inkwell::values::IntValue<'ctx>> {
1062+
let clipped = self.xlen_type().const_all_ones();
1063+
let is_overflow = self.builder().build_int_compare(
1064+
inkwell::IntPredicate::UGT,
1065+
value,
1066+
self.builder()
1067+
.build_int_z_extend(clipped, self.word_type(), "value_clipped")?,
1068+
"is_value_overflow",
1069+
)?;
1070+
let truncated =
1071+
self.builder()
1072+
.build_int_truncate(value, self.xlen_type(), "value_truncated")?;
1073+
Ok(self
1074+
.builder()
1075+
.build_select(is_overflow, clipped, truncated, "value")?
1076+
.into_int_value())
1077+
}
1078+
10571079
/// Build a call to PolkaVM `sbrk` for extending the heap from offset by `size`.
10581080
/// The allocation is aligned to 32 bytes.
10591081
///

crates/llvm-context/src/polkavm/evm/calldata.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ pub fn load<'ctx>(
88
offset: inkwell::values::IntValue<'ctx>,
99
) -> anyhow::Result<inkwell::values::BasicValueEnum<'ctx>> {
1010
let output_pointer = context.build_alloca_at_entry(context.word_type(), "call_data_output");
11-
let offset = context.safe_truncate_int_to_xlen(offset)?;
11+
let offset = context.clip_to_xlen(offset)?;
1212

1313
context.build_runtime_call(
1414
revive_runtime_api::polkavm_imports::CALL_DATA_LOAD,
@@ -40,9 +40,9 @@ pub fn copy<'ctx>(
4040
source_offset: inkwell::values::IntValue<'ctx>,
4141
size: inkwell::values::IntValue<'ctx>,
4242
) -> anyhow::Result<()> {
43-
let source_offset = context.safe_truncate_int_to_xlen(source_offset)?;
44-
let size = context.safe_truncate_int_to_xlen(size)?;
45-
let destination_offset = context.safe_truncate_int_to_xlen(destination_offset)?;
43+
let source_offset = context.clip_to_xlen(source_offset)?;
44+
let size = context.clip_to_xlen(size)?;
45+
let destination_offset = context.clip_to_xlen(destination_offset)?;
4646
let output_pointer = context.build_heap_gep(destination_offset, size)?;
4747

4848
context.build_runtime_call(

0 commit comments

Comments
 (0)