Summary
A write-after-write vulnerability may arise if multiple write operations target the same memory location or storage variable without proper synchronization. Incorrect calculation of offsets and lengths in the assembly code could lead to improper parsing of input data, resulting in the function returning incorrect or malformed data.
Vulnerability Details
A write-after-write vulnerability potentially occurs in this function
function parseEnableModeData(bytes calldata packedData)
internal
pure
returns (
uint256 moduleType,
bytes calldata moduleInitData,
bytes calldata enableModeSignature,
bytes calldata userOpSignature
)
{
uint256 p;
assembly {
p := packedData.offset
moduleType := calldataload(p)
moduleInitData.length := shr(224, calldataload(add(p, 0x20)))
moduleInitData.offset := add(p, 0x24)
p := add(moduleInitData.offset, moduleInitData.length)
enableModeSignature.length := shr(224, calldataload(p))
enableModeSignature.offset := add(p, 0x04)
p := sub(add(enableModeSignature.offset, enableModeSignature.length), packedData.offset)
}
userOpSignature = packedData[p:];
}
Test for parseEnableModeData
pragma solidity 0.8.26;
import "forge-std/Test.sol";
import "./LocalCallDataParserLib.sol";
contract LocalCallDataParserLibTest is Test {
using LocalCallDataParserLib for bytes;
function testParseEnableModeData() public {
bytes memory packedData = hex"0000000000000000000000000000000000000000000000000000000000000001"
hex"0000000000000000000000000000000000000000000000000000000000000020"
hex"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
hex"0000000000000000000000000000000000000000000000000000000000000010"
hex"abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef"
hex"abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef";
(uint256 moduleType, bytes memory moduleInitData, bytes memory enableModeSignature, bytes memory userOpSignature) = packedData.parseEnableModeData();
assertEq(moduleType, 1);
assertEq(moduleInitData.length, 32);
assertEq(enableModeSignature.length, 16);
assertEq(userOpSignature.length, 32);
assertEq(keccak256(moduleInitData), keccak256(hex"1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"));
assertEq(keccak256(enableModeSignature), keccak256(hex"abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef"));
assertEq(keccak256(userOpSignature), keccak256(hex"abcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdefabcdef"));
}
}
Impact
The function's incorrect behavior or malformed data could affect the logic of the contract that relies on correctly parsed data. This could cause unexpected behavior in the application.
Tools Used
Manual Review
Recommendations
function parseEnableModeData(bytes calldata packedData)
internal
pure
returns (
uint256 moduleType,
bytes calldata moduleInitData,
bytes calldata enableModeSignature,
bytes calldata userOpSignature
)
{
uint256 p;
uint256 moduleInitDataLength;
uint256 enableModeSignatureLength;
assembly {
p := packedData.offset
moduleType := calldataload(p)
moduleInitDataLength := shr(224, calldataload(add(p, 0x20)))
moduleInitData.offset := add(p, 0x24)
p := add(moduleInitData.offset, moduleInitDataLength)
enableModeSignatureLength := shr(224, calldataload(p))
enableModeSignature.offset := add(p, 0x04)
p := add(enableModeSignature.offset, enableModeSignatureLength)
}
moduleInitData = packedData[moduleInitData.offset : moduleInitData.offset + moduleInitDataLength];
enableModeSignature = packedData[enableModeSignature.offset : enableModeSignature.offset + enableModeSignatureLength];
userOpSignature = packedData[p : packedData.length];
}
Temporary variables moduleInitDataLength and enableModeSignatureLength are utilized to store the lengths of the respective data fields read from calldata, preventing premature overwriting of memory locations.
Each step of reading and writing from calldata is isolated. Initially, the lengths and offsets of moduleInitData and enableModeSignature are read and stored in temporary variables. These values are then assigned outside the assembly block to ensure proper handling.
Assembly constructs are used correctly to manage calldata and memory operations safely. Offsets are calculated accurately, and lengths are assigned outside the assembly block to avoid potential issues.
