Skip to content

Commit 43c59b5

Browse files
committed
evmasm: Fix unchecked overflow in legacy codegen array size computation
1 parent 4ecc97e commit 43c59b5

7 files changed

Lines changed: 59 additions & 5 deletions

File tree

Changelog.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ Language Features:
44

55
Compiler Features:
66

7+
Important Bugfixes:
8+
* Legacy Code Generator: Fix unchecked multiplication overflow when computing the storage size of dynamic arrays during deletion, which could result in ``delete`` silently leaving stale data in storage.
9+
710
Bugfixes:
811

912

docs/bugs.json

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,17 @@
11
[
2+
{
3+
"uid": "SOL-2026-2",
4+
"name": "EvmasmCodegenUncheckedArraySizeOverflow",
5+
"summary": "Deleting a dynamic array of large static arrays could silently skip storage clearing due to an unchecked multiplication overflow in the evmasm code generator.",
6+
"description": "When the legacy (evmasm) code generator computes the total number of storage slots occupied by an array, it multiplies the array length by the storage size of its base type. This multiplication was performed without an overflow check, so when the product exceeded ``2**256``, the result would wrap to a small value (or zero). This caused the subsequent clearing loop to process fewer slots than necessary, leaving stale data in storage. The bug could be triggered by using the ``delete`` operator on a dynamic storage array whose base type is large enough for the product to overflow. The via-IR pipeline was not affected, because it already used overflow-checked arithmetic for this computation. With the fix, the legacy code generator now reverts with an arithmetic overflow panic in this situation, matching the via-IR behavior.",
7+
"link": "",
8+
"introduced": "0.1.0",
9+
"fixed": "0.8.35",
10+
"severity": "low",
11+
"conditions": {
12+
"viaIR": false
13+
}
14+
},
215
{
316
"uid": "SOL-2026-1",
417
"name": "TransientStorageClearingHelperCollision",

libsolidity/codegen/ArrayUtils.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,14 @@ void ArrayUtils::convertLengthToSize(ArrayType const& _arrayType, bool _pad) con
641641
}
642642
}
643643
else
644-
m_context << _arrayType.baseType()->storageSize() << Instruction::MUL;
644+
{
645+
m_context << _arrayType.baseType()->storageSize();
646+
m_context.callYulFunction(
647+
m_context.utilFunctions().overflowCheckedIntMulFunction(*TypeProvider::uint256()),
648+
2,
649+
1
650+
);
651+
}
645652
}
646653
else
647654
{

test/libsolidity/semanticTests/array/copying/array_copy_including_array.sol

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ contract c {
3636
// ----
3737
// test() -> 0x02000202
3838
// gas irOptimized: 4560468
39-
// gas legacy: 4536539
39+
// gas legacy: 4538699
4040
// gas legacyOptimized: 4456732
4141
// storageEmpty -> 1
4242
// clear() -> 0, 0
4343
// gas irOptimized: 4488719
44-
// gas legacy: 4407759
44+
// gas legacy: 4409439
4545
// gas legacyOptimized: 4385068
4646
// storageEmpty -> 1
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
contract C {
2+
uint256[2**255][] data;
3+
4+
function fill() public {
5+
data.push();
6+
data.push();
7+
data[0][0] = 111;
8+
data[1][0] = 222;
9+
}
10+
11+
function get(uint256 idx) public view returns (uint256) {
12+
return data[idx][0];
13+
}
14+
15+
function clear() public {
16+
delete data;
17+
}
18+
19+
function length() public view returns (uint256) {
20+
return data.length;
21+
}
22+
}
23+
// ----
24+
// fill() ->
25+
// get(uint256): 0 -> 111
26+
// get(uint256): 1 -> 222
27+
// length() -> 2
28+
// clear() -> FAILURE, hex"4e487b71", 0x11
29+
// get(uint256): 0 -> 111
30+
// get(uint256): 1 -> 222
31+
// length() -> 2

test/libsolidity/semanticTests/storage/storage_boundary_struct_array_mixed_types.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ contract C {
159159
// gas legacyOptimized: 112505
160160
// deleteBoundaryArray()
161161
// gas irOptimized: 177968
162-
// gas legacy: 180995
162+
// gas legacy: 181187
163163
// gas legacyOptimized: 178182
164164
// canaryValue() -> 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
165165
// boundaryArray() -> 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0

test/libsolidity/semanticTests/storage/storage_boundary_struct_array_multislot.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ contract C {
121121
// destArray() -> 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60
122122
// deleteBoundaryArray()
123123
// gas irOptimized: 137824
124-
// gas legacy: 137971
124+
// gas legacy: 138163
125125
// gas legacyOptimized: 137750
126126
// canaryValue() -> 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
127127
// boundaryArray() -> 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0

0 commit comments

Comments
 (0)