Skip to content

Commit 56f49e6

Browse files
mmv08teryanarmenUxio0frangiormeissner
authored
Feature: Mark assembly blocks as memory-safe (#545)
* add setup and rules about modules * fix run script * use solc7.6 for fv * Mark assembly blocks as memory-safe * use solidity 0.8.19 for github action benchmark * Update makefile * fix the harness patch * properties doc skeleton * properties notes * use 10m optimizer runs * Write calldata/return data to the memory allocated via the free memory pointer * memory-safe simulateAndRevert * Update CLA github action to v2.3.0 * add certora workflow * Fix changelog mention of createChainSpecificProxyWithNonce * fix script path * use cvl2 * Remove gasleft in setupModules, add erc4337 compatibility test * Fix typechecking in test files (#573) * verify that guard can only be updated through setGuard * Verify functions that may change the fallback handler address (#566) * Add an invariant for singleton address (#565) * Add an optimistic assumption about DELEGATECALL, update nonce monotonicity rule (#574) * Pump version to 1.4.1 (#579) * Formal verification: native token balance updates (#582) * Add an optimistic assumption about DELEGATECALL, update nonce monotonicity rule * Add a rule for token balance * Add more rules for native token balance transition * Fix addresses for 1.4.1 in changelog (#590) * Fix certora CI action * Formal verification: No message can be signed through the core contract (#583) * Add a rule for no signed messages * Add a rule for no signed messages * fix munged patch --------- Co-authored-by: teryanarmen <61996358+teryanarmen@users.noreply.github.com> Co-authored-by: Uxio Fuentefria <Uxio0@users.noreply.github.com> Co-authored-by: Francisco Giordano <fg@frang.io> Co-authored-by: Richard Meissner <rmeissner@users.noreply.github.com> Co-authored-by: Mikhail Mikheev <mmv@pop-os.localdomain>
1 parent 2fb54f1 commit 56f49e6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+1020
-181
lines changed

.env.sample

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,8 @@ INFURA_KEY=""
44
# Used for custom network
55
NODE_URL=""
66
ETHERSCAN_API_KEY=""
7+
# (Optional) Used to run ERC-4337 compatibility test. MNEMONIC is also required.
8+
ERC4337_TEST_BUNDLER_URL=
9+
ERC4337_TEST_NODE_URL=
10+
ERC4337_TEST_SINGLETON_ADDRESS=
11+
ERC4337_TEST_SAFE_FACTORY_ADDRESS=

.github/workflows/certora.yml

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
name: certora
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
pull_request:
8+
branches:
9+
- main
10+
11+
workflow_dispatch:
12+
13+
jobs:
14+
verify:
15+
runs-on: ubuntu-latest
16+
17+
steps:
18+
- uses: actions/checkout@v3
19+
20+
- name: Install python
21+
uses: actions/setup-python@v4
22+
with: { python-version: 3.11 }
23+
24+
- name: Install java
25+
uses: actions/setup-java@v3
26+
with: { java-version: "17", java-package: jre, distribution: semeru }
27+
28+
- name: Install certora cli
29+
run: pip install -Iv certora-cli
30+
31+
- name: Install solc
32+
run: |
33+
wget https://github.com/ethereum/solidity/releases/download/v0.7.6/solc-static-linux
34+
chmod +x solc-static-linux
35+
sudo mv solc-static-linux /usr/local/bin/solc7.6
36+
37+
- name: Verify rule ${{ matrix.rule }}
38+
run: |
39+
cd certora
40+
touch applyHarness.patch
41+
make munged
42+
cd ..
43+
echo "key length" ${#CERTORAKEY}
44+
./certora/scripts/${{ matrix.rule }}
45+
env:
46+
CERTORAKEY: ${{ secrets.CERTORA_KEY }}
47+
48+
strategy:
49+
fail-fast: false
50+
max-parallel: 16
51+
matrix:
52+
rule:
53+
- verifySafe.sh

.github/workflows/ci.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ jobs:
6262
strategy:
6363
fail-fast: false
6464
matrix:
65-
solidity: ["0.7.6", "0.8.2"]
65+
solidity: ["0.7.6", "0.8.19"]
6666
include:
67-
- solidity: "0.8.2"
68-
settings: '{"viaIR":true,"optimizer":{"enabled":true,"runs":10000}}'
67+
- solidity: "0.8.19"
68+
settings: '{"viaIR":true,"optimizer":{"enabled":true,"runs":1000000}}'
6969
env:
7070
SOLIDITY_VERSION: ${{ matrix.solidity }}
7171
SOLIDITY_SETTINGS: ${{ matrix.settings }}

.github/workflows/cla.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ jobs:
1212
- name: "CLA Assistant"
1313
if: (github.event.comment.body == 'recheck' || github.event.comment.body == 'I have read the CLA Document and I hereby sign the CLA') || github.event_name == 'pull_request_target'
1414
# Beta Release
15-
uses: cla-assistant/github-action@v2.1.3-beta
15+
uses: cla-assistant/github-action@v2.3.0
1616
env:
1717
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
1818
# the below token should have repo scope and must be manually added by you in the repository's secret
@@ -33,4 +33,4 @@ jobs:
3333
#custom-pr-sign-comment: 'The signature to be committed in order to sign the CLA'
3434
#custom-allsigned-prcomment: 'pull request comment when all contributors has signed, defaults to **CLA Assistant Lite bot** All Contributors have signed the CLA.'
3535
#lock-pullrequest-aftermerge: false - if you don't want this bot to automatically lock the pull request after merging (default - true)
36-
#use-dco-flag: true - If you are using DCO instead of CLA
36+
#use-dco-flag: true - If you are using DCO instead of CLA

.gitignore

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,9 @@ bin/
1111
solc
1212
coverage/
1313
coverage.json
14-
yarn-error.log
14+
yarn-error.log
15+
16+
# Certora Formal Verification related files
17+
.certora_internal
18+
.certora_recent_jobs.json
19+
.zip-output-url.txt

CHANGELOG.md

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,52 @@
22

33
This changelog only contains changes starting from version 1.3.0
44

5+
# Version 1.4.1
6+
7+
## Compiler settings
8+
9+
Solidity compiler: [0.7.6](https://github.com/ethereum/solidity/releases/tag/v0.7.6) (for more info see issue [#251](https://github.com/safe-global/safe-contracts/issues/251))
10+
11+
Solidity optimizer: `disabled`
12+
13+
## Expected addresses with [Safe Singleton Factory](https://github.com/safe-global/safe-singleton-factory)
14+
15+
### Core contracts
16+
17+
- `Safe` at `0x41675C099F32341bf84BFc5382aF534df5C7461a`
18+
- `SafeL2` at `0x29fcB43b46531BcA003ddC8FCB67FFE91900C762`
19+
20+
### Factory contracts
21+
22+
- `SafeProxyFactory` at `0x4e1DCf7AD4e460CfD30791CCC4F9c8a4f820ec67`
23+
24+
### Handler contracts
25+
26+
- `TokenCallbackHandler` at `0xeDCF620325E82e3B9836eaaeFdc4283E99Dd7562`
27+
- `CompatibilityFallbackHandler` at `0xfd0732Dc9E303f09fCEf3a7388Ad10A83459Ec99`
28+
29+
### Lib contracts
30+
31+
- `MultiSend` at `0x38869bf66a61cF6bDB996A6aE40D5853Fd43B526`
32+
- `MultiSendCallOnly` at `0x9641d764fc13c8B624c04430C7356C1C7C8102e2`
33+
- `CreateCall` at `0x9b35Af71d77eaf8d7e40252370304687390A1A52`
34+
- `SignMessageLib` at `0xd53cd0aB83D845Ac265BE939c57F53AD838012c9`
35+
36+
### Storage reader contracts
37+
38+
- `SimulateTxAccessor` at `0x3d4BA2E0884aa488718476ca2FB8Efc291A46199`
39+
40+
## Changes
41+
42+
### Bugfixes
43+
44+
#### Remove `gasleft()` usage in `setupModules`
45+
46+
Issue: [#568](https://github.com/safe-global/safe-contracts/issues/568)
47+
48+
`setupModules` made a call to `gasleft()` that is invalid in the ERC-4337 standard. The call was replaced with `type(uint256).max` to forward all the available gas instead.
49+
50+
551
# Version 1.4.0
652

753
## Compiler settings
@@ -107,7 +153,7 @@ This method uses the `CREATE` opcode, which is not counterfactual for a specific
107153

108154
If the initializer data is provided, the Factory now checks that the Singleton contract exists and the success of the call to avoid a proxy being deployed uninitialized
109155

110-
#### Add `createNetworkSpecificProxy`
156+
#### Add `createChainSpecificProxyWithNonce`
111157

112158
This method will use the chain id in the `CREATE2` salt; therefore, deploying a proxy to the same address on other networks is impossible.
113159
This method should enable the creation of proxies that should exist only on one network (e.g. specific governance or admin accounts)

README.md

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,29 @@ Usage
1515
yarn
1616
```
1717

18-
### Run all tests:
18+
### Testing
19+
20+
To run the tests:
1921

2022
```bash
2123
yarn build
2224
yarn test
2325
```
2426

27+
Optionally, if you want to run the ERC-4337 compatibility test, it uses a live bundler and node, so it contains some pre-requisites:
28+
29+
1. Define the environment variables:
30+
31+
```
32+
ERC4337_TEST_BUNDLER_URL=
33+
ERC4337_TEST_NODE_URL=
34+
ERC4337_TEST_SINGLETON_ADDRESS=
35+
ERC4337_TEST_SAFE_FACTORY_ADDRESS=
36+
MNEMONIC=
37+
```
38+
39+
2. Pre-fund the executor account derived from the mnemonic with some Native Token to cover the deployment of an ERC4337 module and the pre-fund of the Safe for the test operation.
40+
2541
### Deployments
2642

2743
A collection of the different Safe contract deployments and their addresses can be found in the [Safe deployments](https://github.com/safe-global/safe-deployments) repository.
@@ -92,7 +108,7 @@ Documentation
92108

93109
Audits/ Formal Verification
94110
---------
95-
- [for Version 1.4.0 by Ackee Blockchain](docs/audit_1_4_0.md)
111+
- [for Version 1.4.0/1.4.1 by Ackee Blockchain](docs/audit_1_4_0.md)
96112
- [for Version 1.3.0 by G0 Group](docs/audit_1_3_0.md)
97113
- [for Version 1.2.0 by G0 Group](docs/audit_1_2_0.md)
98114
- [for Version 1.1.1 by G0 Group](docs/audit_1_1_1.md)

certora/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
munged

certora/Makefile

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
default: help
2+
3+
PATCH = applyHarness.patch
4+
CONTRACTS_DIR = ../contracts
5+
MUNGED_DIR = munged
6+
7+
help:
8+
@echo "usage:"
9+
@echo " make clean: remove all generated files (those ignored by git)"
10+
@echo " make munged: create $(MUNGED_DIR) directory by applying the patch file to $(CONTRACTS_DIR)"
11+
@echo " make record: record a new patch file capturing the differences between $(CONTRACTS_DIR) and $(MUNGED_DIR)"
12+
13+
munged: $(wildcard $(CONTRACTS_DIR)/*.sol) $(PATCH)
14+
rm -rf $@
15+
cp -r $(CONTRACTS_DIR) $@
16+
patch -p0 -d $@ < $(PATCH)
17+
18+
record:
19+
diff -druN $(CONTRACTS_DIR) $(MUNGED_DIR) | sed 's+../contracts/++g' | sed 's+munged/++g' > $(PATCH)
20+
21+
refresh: munged record
22+
23+
clean:
24+
git clean -fdX
25+
touch $(PATCH)

certora/applyHarness.patch

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
diff -druN base/Executor.sol base/Executor.sol
2+
--- base/Executor.sol 2023-06-22 14:42:39.769357540 +0200
3+
+++ base/Executor.sol 2023-06-22 15:34:09.211725615 +0200
4+
@@ -26,11 +26,13 @@
5+
uint256 txGas
6+
) internal returns (bool success) {
7+
if (operation == Enum.Operation.DelegateCall) {
8+
+ // MUNGED lets just be a bit more optimistic, `execute` does nothing for `DELEGATECALL` and always returns true
9+
// solhint-disable-next-line no-inline-assembly
10+
/// @solidity memory-safe-assembly
11+
- assembly {
12+
- success := delegatecall(txGas, to, add(data, 0x20), mload(data), 0, 0)
13+
- }
14+
+ // assembly {
15+
+ // success := delegatecall(txGas, to, add(data, 0x20), mload(data), 0, 0)
16+
+ // }
17+
+ return true;
18+
} else {
19+
// solhint-disable-next-line no-inline-assembly
20+
/// @solidity memory-safe-assembly
21+
diff -druN Safe.sol Safe.sol
22+
--- Safe.sol 2023-06-22 14:43:53.738861263 +0200
23+
+++ Safe.sol 2023-06-22 15:28:00.436277677 +0200
24+
@@ -76,7 +76,7 @@
25+
* so we create a Safe with 0 owners and threshold 1.
26+
* This is an unusable Safe, perfect for the singleton
27+
*/
28+
- threshold = 1;
29+
+ // threshold = 1; MUNGED: remove and add to constructor of the harness
30+
}
31+
32+
/**

0 commit comments

Comments
 (0)