Mark assembly as `memory safe` (#107)

* Adds `memory-safe` tag to assembly sections in `BytesLib`.

This improves codegen in the compiler and reduces the chance of
untraceable internal codegen errors when dealing with stack variables
that can't be referenced with an instruction.

Also, it removes unused functions in `BytesLib` to reduce code surface.

* Renames functions and contracts in reentrancy negative test.

* Adds `unchecked` block for overflow check.
This commit is contained in:
scnale 2023-02-27 19:59:00 -03:00 committed by GitHub
parent f7de3d649d
commit 24d682bca4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 15 additions and 331 deletions

View File

@ -4,218 +4,20 @@
* @author Gonçalo <goncalo.sa@consensys.net>
*
* @dev Bytes tightly packed arrays utility library for ethereum contracts written in Solidity.
* The library lets you concatenate, slice and type cast bytes arrays both in memory and storage.
* This is a reduced version of the library.
*/
pragma solidity >=0.8.0 <0.9.0;
library BytesLib {
function concat(bytes memory _preBytes, bytes memory _postBytes) internal pure returns (bytes memory) {
bytes memory tempBytes;
assembly {
// Get a location of some free memory and store it in tempBytes as
// Solidity does for memory variables.
tempBytes := mload(0x40)
// Store the length of the first bytes array at the beginning of
// the memory for tempBytes.
let length := mload(_preBytes)
mstore(tempBytes, length)
// Maintain a memory counter for the current write location in the
// temp bytes array by adding the 32 bytes for the array length to
// the starting location.
let mc := add(tempBytes, 0x20)
// Stop copying when the memory counter reaches the length of the
// first bytes array.
let end := add(mc, length)
for {
// Initialize a copy counter to the start of the _preBytes data,
// 32 bytes into its memory.
let cc := add(_preBytes, 0x20)
} lt(mc, end) {
// Increase both counters by 32 bytes each iteration.
mc := add(mc, 0x20)
cc := add(cc, 0x20)
} {
// Write the _preBytes data into the tempBytes memory 32 bytes
// at a time.
mstore(mc, mload(cc))
}
// Add the length of _postBytes to the current length of tempBytes
// and store it as the new length in the first 32 bytes of the
// tempBytes memory.
length := mload(_postBytes)
mstore(tempBytes, add(length, mload(tempBytes)))
// Move the memory counter back from a multiple of 0x20 to the
// actual end of the _preBytes data.
mc := end
// Stop copying when the memory counter reaches the new combined
// length of the arrays.
end := add(mc, length)
for { let cc := add(_postBytes, 0x20) } lt(mc, end) {
mc := add(mc, 0x20)
cc := add(cc, 0x20)
} { mstore(mc, mload(cc)) }
// Update the free-memory pointer by padding our last write location
// to 32 bytes: add 31 bytes to the end of tempBytes to move to the
// next 32 byte block, then round down to the nearest multiple of
// 32. If the sum of the length of the two arrays is zero then add
// one before rounding down to leave a blank 32 bytes (the length block with 0).
mstore(
0x40,
and(
add(add(end, iszero(add(length, mload(_preBytes)))), 31),
not(31) // Round down to the nearest 32 bytes.
)
)
}
return tempBytes;
}
function concatStorage(bytes storage _preBytes, bytes memory _postBytes) internal {
assembly {
// Read the first 32 bytes of _preBytes storage, which is the length
// of the array. (We don't need to use the offset into the slot
// because arrays use the entire slot.)
let fslot := sload(_preBytes.slot)
// Arrays of 31 bytes or less have an even value in their slot,
// while longer arrays have an odd value. The actual length is
// the slot divided by two for odd values, and the lowest order
// byte divided by two for even values.
// If the slot is even, bitwise and the slot with 255 and divide by
// two to get the length. If the slot is odd, bitwise and the slot
// with -1 and divide by two.
let slength := div(and(fslot, sub(mul(0x100, iszero(and(fslot, 1))), 1)), 2)
let mlength := mload(_postBytes)
let newlength := add(slength, mlength)
// slength can contain both the length and contents of the array
// if length < 32 bytes so let's prepare for that
// v. http://solidity.readthedocs.io/en/latest/miscellaneous.html#layout-of-state-variables-in-storage
switch add(lt(slength, 32), lt(newlength, 32))
case 2 {
// Since the new array still fits in the slot, we just need to
// update the contents of the slot.
// uint256(bytes_storage) = uint256(bytes_storage) + uint256(bytes_memory) + new_length
sstore(
_preBytes.slot,
// all the modifications to the slot are inside this
// next block
add(
// we can just add to the slot contents because the
// bytes we want to change are the LSBs
fslot,
add(
mul(
div(
// load the bytes from memory
mload(add(_postBytes, 0x20)),
// zero all bytes to the right
exp(0x100, sub(32, mlength))
),
// and now shift left the number of bytes to
// leave space for the length in the slot
exp(0x100, sub(32, newlength))
),
// increase length by the double of the memory
// bytes length
mul(mlength, 2)
)
)
)
}
case 1 {
// The stored value fits in the slot, but the combined value
// will exceed it.
// get the keccak hash to get the contents of the array
mstore(0x0, _preBytes.slot)
let sc := add(keccak256(0x0, 0x20), div(slength, 32))
// save new length
sstore(_preBytes.slot, add(mul(newlength, 2), 1))
// The contents of the _postBytes array start 32 bytes into
// the structure. Our first read should obtain the `submod`
// bytes that can fit into the unused space in the last word
// of the stored array. To get this, we read 32 bytes starting
// from `submod`, so the data we read overlaps with the array
// contents by `submod` bytes. Masking the lowest-order
// `submod` bytes allows us to add that value directly to the
// stored value.
let submod := sub(32, slength)
let mc := add(_postBytes, submod)
let end := add(_postBytes, mlength)
let mask := sub(exp(0x100, submod), 1)
sstore(
sc,
add(
and(fslot, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff00),
and(mload(mc), mask)
)
)
for {
mc := add(mc, 0x20)
sc := add(sc, 1)
} lt(mc, end) {
sc := add(sc, 1)
mc := add(mc, 0x20)
} { sstore(sc, mload(mc)) }
mask := exp(0x100, sub(mc, end))
sstore(sc, mul(div(mload(mc), mask), mask))
}
default {
// get the keccak hash to get the contents of the array
mstore(0x0, _preBytes.slot)
// Start copying to the last used word of the stored array.
let sc := add(keccak256(0x0, 0x20), div(slength, 32))
// save new length
sstore(_preBytes.slot, add(mul(newlength, 2), 1))
// Copy over the first `submod` bytes of the new data as in
// case 1 above.
let slengthmod := mod(slength, 32)
let mlengthmod := mod(mlength, 32)
let submod := sub(32, slengthmod)
let mc := add(_postBytes, submod)
let end := add(_postBytes, mlength)
let mask := sub(exp(0x100, submod), 1)
sstore(sc, add(sload(sc), and(mload(mc), mask)))
for {
sc := add(sc, 1)
mc := add(mc, 0x20)
} lt(mc, end) {
sc := add(sc, 1)
mc := add(mc, 0x20)
} { sstore(sc, mload(mc)) }
mask := exp(0x100, sub(mc, end))
sstore(sc, mul(div(mload(mc), mask), mask))
}
}
}
function slice(bytes memory _bytes, uint256 _start, uint256 _length) internal pure returns (bytes memory) {
require(_length + 31 >= _length, "slice_overflow");
unchecked {
require(_length + 31 >= _length, "slice_overflow");
}
require(_bytes.length >= _start + _length, "slice_outOfBounds");
bytes memory tempBytes;
assembly {
assembly ("memory-safe") {
switch iszero(_length)
case 0 {
// Get a location of some free memory and store it in tempBytes as
@ -272,7 +74,7 @@ library BytesLib {
require(_bytes.length >= _start + 20, "toAddress_outOfBounds");
address tempAddress;
assembly {
assembly ("memory-safe") {
tempAddress := div(mload(add(add(_bytes, 0x20), _start)), 0x1000000000000000000000000)
}
@ -283,7 +85,7 @@ library BytesLib {
require(_bytes.length >= _start + 1, "toUint8_outOfBounds");
uint8 tempUint;
assembly {
assembly ("memory-safe") {
tempUint := mload(add(add(_bytes, 0x1), _start))
}
@ -294,7 +96,7 @@ library BytesLib {
require(_bytes.length >= _start + 2, "toUint16_outOfBounds");
uint16 tempUint;
assembly {
assembly ("memory-safe") {
tempUint := mload(add(add(_bytes, 0x2), _start))
}
@ -305,7 +107,7 @@ library BytesLib {
require(_bytes.length >= _start + 4, "toUint32_outOfBounds");
uint32 tempUint;
assembly {
assembly ("memory-safe") {
tempUint := mload(add(add(_bytes, 0x4), _start))
}
@ -316,29 +118,18 @@ library BytesLib {
require(_bytes.length >= _start + 8, "toUint64_outOfBounds");
uint64 tempUint;
assembly {
assembly ("memory-safe") {
tempUint := mload(add(add(_bytes, 0x8), _start))
}
return tempUint;
}
function toUint96(bytes memory _bytes, uint256 _start) internal pure returns (uint96) {
require(_bytes.length >= _start + 12, "toUint96_outOfBounds");
uint96 tempUint;
assembly {
tempUint := mload(add(add(_bytes, 0xc), _start))
}
return tempUint;
}
function toUint128(bytes memory _bytes, uint256 _start) internal pure returns (uint128) {
require(_bytes.length >= _start + 16, "toUint128_outOfBounds");
uint128 tempUint;
assembly {
assembly ("memory-safe") {
tempUint := mload(add(add(_bytes, 0x10), _start))
}
@ -349,7 +140,7 @@ library BytesLib {
require(_bytes.length >= _start + 32, "toUint256_outOfBounds");
uint256 tempUint;
assembly {
assembly ("memory-safe") {
tempUint := mload(add(add(_bytes, 0x20), _start))
}
@ -360,117 +151,10 @@ library BytesLib {
require(_bytes.length >= _start + 32, "toBytes32_outOfBounds");
bytes32 tempBytes32;
assembly {
assembly ("memory-safe") {
tempBytes32 := mload(add(add(_bytes, 0x20), _start))
}
return tempBytes32;
}
function equal(bytes memory _preBytes, bytes memory _postBytes) internal pure returns (bool) {
bool success = true;
assembly {
let length := mload(_preBytes)
// if lengths don't match the arrays are not equal
switch eq(length, mload(_postBytes))
case 1 {
// cb is a circuit breaker in the for loop since there's
// no said feature for inline assembly loops
// cb = 1 - don't breaker
// cb = 0 - break
let cb := 1
let mc := add(_preBytes, 0x20)
let end := add(mc, length)
for { let cc := add(_postBytes, 0x20) }
// the next line is the loop condition:
// while(uint256(mc < end) + cb == 2)
eq(add(lt(mc, end), cb), 2) {
mc := add(mc, 0x20)
cc := add(cc, 0x20)
} {
// if any of these checks fails then arrays are not equal
if iszero(eq(mload(mc), mload(cc))) {
// unsuccess:
success := 0
cb := 0
}
}
}
default {
// unsuccess:
success := 0
}
}
return success;
}
function equalStorage(bytes storage _preBytes, bytes memory _postBytes) internal view returns (bool) {
bool success = true;
assembly {
// we know _preBytes_offset is 0
let fslot := sload(_preBytes.slot)
// Decode the length of the stored array like in concatStorage().
let slength := div(and(fslot, sub(mul(0x100, iszero(and(fslot, 1))), 1)), 2)
let mlength := mload(_postBytes)
// if lengths don't match the arrays are not equal
switch eq(slength, mlength)
case 1 {
// slength can contain both the length and contents of the array
// if length < 32 bytes so let's prepare for that
// v. http://solidity.readthedocs.io/en/latest/miscellaneous.html#layout-of-state-variables-in-storage
if iszero(iszero(slength)) {
switch lt(slength, 32)
case 1 {
// blank the last byte which is the length
fslot := mul(div(fslot, 0x100), 0x100)
if iszero(eq(fslot, mload(add(_postBytes, 0x20)))) {
// unsuccess:
success := 0
}
}
default {
// cb is a circuit breaker in the for loop since there's
// no said feature for inline assembly loops
// cb = 1 - don't breaker
// cb = 0 - break
let cb := 1
// get the keccak hash to get the contents of the array
mstore(0x0, _preBytes.slot)
let sc := keccak256(0x0, 0x20)
let mc := add(_postBytes, 0x20)
let end := add(mc, mlength)
// the next line is the loop condition:
// while(uint256(mc < end) + cb == 2)
for {} eq(add(lt(mc, end), cb), 2) {
sc := add(sc, 1)
mc := add(mc, 0x20)
} {
if iszero(eq(sload(sc), mload(mc))) {
// unsuccess:
success := 0
cb := 0
}
}
}
}
}
default {
// unsuccess:
success := 0
}
}
return success;
}
}

View File

@ -39,10 +39,10 @@ contract AttackForwardIntegration is IWormholeReceiver {
// The core relayer could in principle accept the request due to this being the target of the message at the same time as being the refund address.
// Note that, if succesful, this forward request would be processed after the time for processing forwards is past.
// Thus, the request would "linger" in the forward request cache and be attended to in the next delivery.
requestForward(targetChainId, toWormholeFormat(attackerReward));
forward(targetChainId, toWormholeFormat(attackerReward));
}
function requestForward(uint16 targetChain, bytes32 attackerRewardAddress) internal {
function forward(uint16 targetChain, bytes32 attackerRewardAddress) internal {
uint256 maxTransactionFee =
core_relayer.quoteGas(targetChain, SAFE_DELIVERY_GAS_CAPTURE, core_relayer.getDefaultRelayProvider());