From 9d27a398256df57d86f66c1517776e99df160b82 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 11 Feb 2025 12:14:32 +0000 Subject: [PATCH 1/2] Add memsafe scanner --- prep/memory-safe-scan.js | 29 +++++++++++++++++++++++++++++ src/accounts/EIP7702Proxy.sol | 2 +- 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 prep/memory-safe-scan.js diff --git a/prep/memory-safe-scan.js b/prep/memory-safe-scan.js new file mode 100644 index 0000000000..bc2d4521d6 --- /dev/null +++ b/prep/memory-safe-scan.js @@ -0,0 +1,29 @@ +#!/usr/bin/env node +const { readSync, forEachWalkSync, hasAnyPathSequence } = require('./common.js'); + +async function main() { + const pathSequencesToIgnore = ['g', 'ext', 'legacy']; + + const loggedSrcPaths = []; + forEachWalkSync(['src'], srcPath => { + if (!srcPath.match(/\.sol$/i)) return; + if (hasAnyPathSequence(srcPath, pathSequencesToIgnore)) return; + + const src = readSync(srcPath); + const assemblyTagRe = /(\/\/\/\s*?@solidity\s*?memory-safe-assembly\s+?)?assembly\s*?(\(.*?\))?\{/gm; + for (let m = null; (m = assemblyTagRe.exec(src)) !== null; ) { + if ((m[0] + '').indexOf('memory-safe') === -1) { + if (loggedSrcPaths.indexOf(srcPath) === -1) { + loggedSrcPaths.push(srcPath); + console.log(srcPath + ':'); + } + console.log(' line:', src.slice(0, m.index).split(/\n/).length); + } + } + }); +}; + +main().catch(e => { + console.error(e); + process.exit(1); +}); diff --git a/src/accounts/EIP7702Proxy.sol b/src/accounts/EIP7702Proxy.sol index 552e8f8782..95bdfcf6f8 100644 --- a/src/accounts/EIP7702Proxy.sol +++ b/src/accounts/EIP7702Proxy.sol @@ -38,7 +38,7 @@ contract EIP7702Proxy { /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ constructor(address initialImplementation, address initialAdmin) payable { - assembly { + assembly ("memory-safe") { sstore(_ERC1967_IMPLEMENTATION_SLOT, shr(96, shl(96, initialImplementation))) sstore(_ERC1967_ADMIN_SLOT, shr(96, shl(96, initialAdmin))) } From 906bcf0ad63d2519faf58adb416437bb7ffd47e2 Mon Sep 17 00:00:00 2001 From: Vectorized Date: Tue, 11 Feb 2025 12:21:42 +0000 Subject: [PATCH 2/2] Add more memory-safe annotations --- prep/memory-safe-scan.js | 2 +- src/accounts/EIP7702Proxy.sol | 4 +++- src/accounts/ERC6551Proxy.sol | 3 +++ src/tokens/ERC721.sol | 1 + src/tokens/ext/zksync/ERC721.sol | 1 + src/utils/DynamicArrayLib.sol | 2 ++ src/utils/ECDSA.sol | 8 ++++---- src/utils/LibBytes.sol | 2 ++ src/utils/LibString.sol | 1 + src/utils/g/DynamicArrayLib.sol | 2 ++ src/utils/g/LibBytes.sol | 2 ++ src/utils/g/LibString.sol | 1 + 12 files changed, 23 insertions(+), 6 deletions(-) diff --git a/prep/memory-safe-scan.js b/prep/memory-safe-scan.js index bc2d4521d6..c63c3edb15 100644 --- a/prep/memory-safe-scan.js +++ b/prep/memory-safe-scan.js @@ -2,7 +2,7 @@ const { readSync, forEachWalkSync, hasAnyPathSequence } = require('./common.js'); async function main() { - const pathSequencesToIgnore = ['g', 'ext', 'legacy']; + const pathSequencesToIgnore = ['g', 'legacy']; const loggedSrcPaths = []; forEachWalkSync(['src'], srcPath => { diff --git a/src/accounts/EIP7702Proxy.sol b/src/accounts/EIP7702Proxy.sol index 95bdfcf6f8..e94d6bdedc 100644 --- a/src/accounts/EIP7702Proxy.sol +++ b/src/accounts/EIP7702Proxy.sol @@ -38,7 +38,8 @@ contract EIP7702Proxy { /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/ constructor(address initialImplementation, address initialAdmin) payable { - assembly ("memory-safe") { + /// @solidity memory-safe-assembly + assembly { sstore(_ERC1967_IMPLEMENTATION_SLOT, shr(96, shl(96, initialImplementation))) sstore(_ERC1967_ADMIN_SLOT, shr(96, shl(96, initialAdmin))) } @@ -50,6 +51,7 @@ contract EIP7702Proxy { fallback() external payable virtual { uint256 s = __self; + /// @solidity memory-safe-assembly assembly { mstore(0x40, returndatasize()) // Optimization trick to change `6040608052` into `3d604052`. // Workflow for calling on the proxy itself. diff --git a/src/accounts/ERC6551Proxy.sol b/src/accounts/ERC6551Proxy.sol index 9b518f3207..eb146756a3 100644 --- a/src/accounts/ERC6551Proxy.sol +++ b/src/accounts/ERC6551Proxy.sol @@ -46,12 +46,14 @@ contract ERC6551Proxy { fallback() external payable virtual { bytes32 implementation; + /// @solidity memory-safe-assembly assembly { mstore(0x40, returndatasize()) // Optimization trick to change `6040608052` into `3d604052`. implementation := sload(_ERC1967_IMPLEMENTATION_SLOT) } if (implementation == bytes32(0)) { implementation = _defaultImplementation; + /// @solidity memory-safe-assembly assembly { // Only initialize if the calldatasize is zero, so that staticcalls to // functions (which will have 4-byte function selectors) won't revert. @@ -60,6 +62,7 @@ contract ERC6551Proxy { if iszero(calldatasize()) { sstore(_ERC1967_IMPLEMENTATION_SLOT, implementation) } } } + /// @solidity memory-safe-assembly assembly { calldatacopy(returndatasize(), returndatasize(), calldatasize()) // forgefmt: disable-next-item diff --git a/src/tokens/ERC721.sol b/src/tokens/ERC721.sol index b880d25dad..19bcc0b547 100644 --- a/src/tokens/ERC721.sol +++ b/src/tokens/ERC721.sol @@ -668,6 +668,7 @@ abstract contract ERC721 { /// /// Emits a {Approval} event. function _approve(address by, address account, uint256 id) internal virtual { + /// @solidity memory-safe-assembly assembly { // Clear the upper 96 bits. let bitmaskAddress := shr(96, not(0)) diff --git a/src/tokens/ext/zksync/ERC721.sol b/src/tokens/ext/zksync/ERC721.sol index b8fa054f23..d73684884b 100644 --- a/src/tokens/ext/zksync/ERC721.sol +++ b/src/tokens/ext/zksync/ERC721.sol @@ -666,6 +666,7 @@ abstract contract ERC721 { /// /// Emits a {Approval} event. function _approve(address by, address account, uint256 id) internal virtual { + /// @solidity memory-safe-assembly assembly { // Clear the upper 96 bits. let bitmaskAddress := shr(96, not(0)) diff --git a/src/utils/DynamicArrayLib.sol b/src/utils/DynamicArrayLib.sol index 71a4924f1d..de98bec0b3 100644 --- a/src/utils/DynamicArrayLib.sol +++ b/src/utils/DynamicArrayLib.sol @@ -347,6 +347,7 @@ library DynamicArrayLib { /// @dev Directly returns `a` without copying. function directReturn(uint256[] memory a) internal pure { + /// @solidity memory-safe-assembly assembly { let retStart := sub(a, 0x20) mstore(retStart, 0x20) @@ -988,6 +989,7 @@ library DynamicArrayLib { /// @dev Directly returns `a` without copying. function directReturn(DynamicArray memory a) internal pure { + /// @solidity memory-safe-assembly assembly { let arrData := mload(a) let retStart := sub(arrData, 0x20) diff --git a/src/utils/ECDSA.sol b/src/utils/ECDSA.sol index a793e1e21e..69d9ec3494 100644 --- a/src/utils/ECDSA.sol +++ b/src/utils/ECDSA.sol @@ -333,7 +333,7 @@ library ECDSA { /// @dev Returns the canonical hash of `signature`. function canonicalHash(bytes memory signature) internal pure returns (bytes32 result) { - // @solidity memory-safe-assembly + /// @solidity memory-safe-assembly assembly { let l := mload(signature) for {} 1 {} { @@ -369,7 +369,7 @@ library ECDSA { pure returns (bytes32 result) { - // @solidity memory-safe-assembly + /// @solidity memory-safe-assembly assembly { for {} 1 {} { mstore(0x00, calldataload(signature.offset)) // `r`. @@ -400,7 +400,7 @@ library ECDSA { /// @dev Returns the canonical hash of `signature`. function canonicalHash(bytes32 r, bytes32 vs) internal pure returns (bytes32 result) { - // @solidity memory-safe-assembly + /// @solidity memory-safe-assembly assembly { mstore(0x00, r) // `r`. let v := add(shr(255, vs), 27) @@ -414,7 +414,7 @@ library ECDSA { /// @dev Returns the canonical hash of `signature`. function canonicalHash(uint8 v, bytes32 r, bytes32 s) internal pure returns (bytes32 result) { - // @solidity memory-safe-assembly + /// @solidity memory-safe-assembly assembly { mstore(0x00, r) // `r`. if iszero(lt(s, _HALF_N_PLUS_1)) { diff --git a/src/utils/LibBytes.sol b/src/utils/LibBytes.sol index 06b3b07c8a..290c57c5c4 100644 --- a/src/utils/LibBytes.sol +++ b/src/utils/LibBytes.sol @@ -645,6 +645,7 @@ library LibBytes { /// @dev Directly returns `a` without copying. function directReturn(bytes memory a) internal pure { + /// @solidity memory-safe-assembly assembly { // Assumes that the bytes does not start from the scratch space. let retStart := sub(a, 0x20) @@ -660,6 +661,7 @@ library LibBytes { /// @dev Directly returns `a` with minimal copying. function directReturn(bytes[] memory a) internal pure { + /// @solidity memory-safe-assembly assembly { let n := mload(a) // `a.length`. let o := add(a, 0x20) // Start of elements in `a`. diff --git a/src/utils/LibString.sol b/src/utils/LibString.sol index 0ed673e1cd..3bfc96b3c1 100644 --- a/src/utils/LibString.sol +++ b/src/utils/LibString.sol @@ -956,6 +956,7 @@ library LibString { /// @dev Directly returns `a` without copying. function directReturn(string memory a) internal pure { + /// @solidity memory-safe-assembly assembly { // Assumes that the string does not start from the scratch space. let retStart := sub(a, 0x20) diff --git a/src/utils/g/DynamicArrayLib.sol b/src/utils/g/DynamicArrayLib.sol index 7ba30ab8f6..2c5ec4be78 100644 --- a/src/utils/g/DynamicArrayLib.sol +++ b/src/utils/g/DynamicArrayLib.sol @@ -351,6 +351,7 @@ library DynamicArrayLib { /// @dev Directly returns `a` without copying. function directReturn(uint256[] memory a) internal pure { + /// @solidity memory-safe-assembly assembly { let retStart := sub(a, 0x20) mstore(retStart, 0x20) @@ -992,6 +993,7 @@ library DynamicArrayLib { /// @dev Directly returns `a` without copying. function directReturn(DynamicArray memory a) internal pure { + /// @solidity memory-safe-assembly assembly { let arrData := mload(a) let retStart := sub(arrData, 0x20) diff --git a/src/utils/g/LibBytes.sol b/src/utils/g/LibBytes.sol index 24088c1c48..0d0877d831 100644 --- a/src/utils/g/LibBytes.sol +++ b/src/utils/g/LibBytes.sol @@ -649,6 +649,7 @@ library LibBytes { /// @dev Directly returns `a` without copying. function directReturn(bytes memory a) internal pure { + /// @solidity memory-safe-assembly assembly { // Assumes that the bytes does not start from the scratch space. let retStart := sub(a, 0x20) @@ -664,6 +665,7 @@ library LibBytes { /// @dev Directly returns `a` with minimal copying. function directReturn(bytes[] memory a) internal pure { + /// @solidity memory-safe-assembly assembly { let n := mload(a) // `a.length`. let o := add(a, 0x20) // Start of elements in `a`. diff --git a/src/utils/g/LibString.sol b/src/utils/g/LibString.sol index 5d6064d368..e89faec068 100644 --- a/src/utils/g/LibString.sol +++ b/src/utils/g/LibString.sol @@ -960,6 +960,7 @@ library LibString { /// @dev Directly returns `a` without copying. function directReturn(string memory a) internal pure { + /// @solidity memory-safe-assembly assembly { // Assumes that the string does not start from the scratch space. let retStart := sub(a, 0x20)