diff --git a/solidity/checks-effects-interactions-gen.py b/solidity/checks-effects-interactions-gen.py new file mode 100644 index 0000000..36f83d5 --- /dev/null +++ b/solidity/checks-effects-interactions-gen.py @@ -0,0 +1,78 @@ +TEMPLATE = '''rules: + - + id: checks-effects-interactions + message: The Checks-Effects-Interactions pattern ensures that validation of the request, and changes to the state variables of the contract, are performed before any interactions take place with other contracts. When contracts are implemented this way, the scope for Re-entrancy Attacks is reduced significantly. + metadata: + category: logic + references: + - https://entethalliance.org/specs/ethtrust-sl/v1/#req-1-use-c-e-i + patterns: + - pattern-inside: + contract $C { + $TYPE $STORAGE; + ... + } + - pattern-either: + %s + languages: + - solidity + severity: ERROR +''' + +PART = '''- pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + %s + ... + } + ''' + +PLACEHOLDER = '' + +operators = '-+/*%^|&' +for op in operators: + for recursion in xrange(3): + PLACEHOLDER += PART % ('$STORAGE%s %s= ...;' % ('[...]' * recursion, op)) + +for op in '+-': + for recursion in xrange(3): + PLACEHOLDER += PART % ('<... $STORAGE%s%s ...>;' % ('[...]' * recursion, op * 2)) + PLACEHOLDER += PART % ('<... %s$STORAGE%s ...>;' % (op * 2, '[...]' * recursion)) + +print TEMPLATE % PLACEHOLDER + +TEMPLATE2 = ''' - + id: checks-effects-interactions-heuristic + message: The Checks-Effects-Interactions pattern ensures that validation of the request, and changes to the state variables of the contract, are performed before any interactions take place with other contracts. When contracts are implemented this way, the scope for Re-entrancy Attacks is reduced significantly. + metadata: + category: logic + references: + - https://entethalliance.org/specs/ethtrust-sl/v1/#req-1-use-c-e-i + patterns: + - pattern-inside: + contract $C { + ... + } + - pattern: + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... $WRITE(...) ...>; + ... + } + - metavariable-regex: + metavariable: $WRITE + regex: (write|change|increase|decrease|delete|remove|add) + languages: + - solidity + severity: ERROR +''' + +#PLACEHOLDER = '' +#for recursion in xrange(3): +# PLACEHOLDER += PART % ('<... $WRITE($STORAGE%s) ...>;' % ('[...]' * recursion)) + +print TEMPLATE2 diff --git a/solidity/checks-effects-interactions.sol b/solidity/checks-effects-interactions.sol new file mode 100644 index 0000000..fd385ac --- /dev/null +++ b/solidity/checks-effects-interactions.sol @@ -0,0 +1,32 @@ +contract Sample { + mapping(address => uint) balances; + + function deposit() public payable { + balances[msg.sender] = msg.value; + } + + // ok: checks-effects-interactions + function withdrawSafe(uint amount) public { + require(balances[msg.sender] >= amount); + balances[msg.sender] -= amount; + msg.sender.transfer(amount); + } + + // ruleid: checks-effects-interactions + function withdrawUnsafe(uint amount) public { + require(balances[msg.sender] >= amount); + msg.sender.transfer(amount); + balances[msg.sender] -= amount; + } + + // ruleid: checks-effects-interactions-heuristic + function withdrawUnsafe2(uint amount) public { + require(balances[msg.sender] >= amount); + msg.sender.transfer(amount); + decreaseBalance(msg.sender, amount); + } + + function decreaseBalance(address who, uint amount) private { + balances[who] -= amount; + } +} \ No newline at end of file diff --git a/solidity/checks-effects-interactions.yaml b/solidity/checks-effects-interactions.yaml new file mode 100644 index 0000000..c322c4e --- /dev/null +++ b/solidity/checks-effects-interactions.yaml @@ -0,0 +1,335 @@ +rules: + - + id: checks-effects-interactions + message: The Checks-Effects-Interactions pattern ensures that validation of the request, and changes to the state variables of the contract, are performed before any interactions take place with other contracts. When contracts are implemented this way, the scope for Re-entrancy Attacks is reduced significantly. + metadata: + category: logic + references: + - https://entethalliance.org/specs/ethtrust-sl/v1/#req-1-use-c-e-i + patterns: + - pattern-inside: + contract $C { + $TYPE $STORAGE; + ... + } + - pattern-either: + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE -= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...] -= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...][...] -= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE += ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...] += ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...][...] += ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE /= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...] /= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...][...] /= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE *= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...] *= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...][...] *= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE %= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...] %= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...][...] %= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE ^= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...] ^= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...][...] ^= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE |= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...] |= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...][...] |= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE &= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...] &= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + $STORAGE[...][...] &= ...; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... $STORAGE++ ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... ++$STORAGE ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... $STORAGE[...]++ ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... ++$STORAGE[...] ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... $STORAGE[...][...]++ ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... ++$STORAGE[...][...] ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... $STORAGE-- ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... --$STORAGE ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... $STORAGE[...]-- ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... --$STORAGE[...] ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... $STORAGE[...][...]-- ...>; + ... + } + - pattern: | + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... --$STORAGE[...][...] ...>; + ... + } + + languages: + - solidity + severity: ERROR + + - + id: checks-effects-interactions-heuristic + message: The Checks-Effects-Interactions pattern ensures that validation of the request, and changes to the state variables of the contract, are performed before any interactions take place with other contracts. When contracts are implemented this way, the scope for Re-entrancy Attacks is reduced significantly. + metadata: + category: logic + references: + - https://entethalliance.org/specs/ethtrust-sl/v1/#req-1-use-c-e-i + patterns: + - pattern-inside: + contract $C { + ... + } + - pattern: + function $F(...) { + ... + $ADDR.$FUNC(...); + ... + <... $WRITE(...) ...>; + ... + } + - metavariable-regex: + metavariable: $WRITE + regex: (write|change|increase|decrease|delete|remove|add) + languages: + - solidity + severity: ERROR + diff --git a/solidity/exact-balance-check.sol b/solidity/exact-balance-check.sol new file mode 100644 index 0000000..700ffe9 --- /dev/null +++ b/solidity/exact-balance-check.sol @@ -0,0 +1,59 @@ +contract Foobar { + function doit1(address ext) { + uint256 bal = IERC20(ext).balanceOf(address(this)); + // ok: exact-balance-check + bal == 1338; + require( + 1==1 && + // ruleid: exact-balance-check + (bal == 1337) || + 1==2, + "Wrong balance!" + ); + // do smth + } + + function doit2(address ext) { + require( + 1==1 && + // ruleid: exact-balance-check + (address(ext).balance == 1337) || + 1==2, + "Wrong balance!" + ); + // do smth + } + + function doit3(address ext) { + require( + 1==1 && + // ruleid: exact-balance-check + (1337 == address(ext).balance) || + 1==2, + "Wrong balance!" + ); + // do smth + } + + function doit_safe(address ext) { + require( + 1==1 && + // ok: exact-balance-check + (IERC20(ext).balanceOf(address(this)) >= 1337) || + 1==2, + "Wrong balance!" + ); + // do smth + } + + function doit2_safe(address ext) { + require( + 1==1 && + // ok: exact-balance-check + (address(ext).balance <= 1337) || + 1==2, + "Wrong balance!" + ); + // do smth + } +} diff --git a/solidity/exact-balance-check.yaml b/solidity/exact-balance-check.yaml new file mode 100644 index 0000000..bdb4c1c --- /dev/null +++ b/solidity/exact-balance-check.yaml @@ -0,0 +1,22 @@ +rules: + - + id: exact-balance-check + message: Testing the balance of an account as a basis for some action has risks associated with unexpected receipt of ether or another token, including tokens deliberately transfered to cause such tests to fail, as an MEV attack. + metadata: + category: logic + references: + - https://entethalliance.org/specs/ethtrust-sl/v1/#req-1-exact-balance-check + mode: taint + pattern-sinks: + - patterns: + - pattern-inside: require(...) + - pattern-either: + - pattern: $X == ... + - pattern: ... == $X + pattern-sources: + - pattern-either: + - pattern: $T.balanceOf($A) + - pattern: $T.balance + languages: + - solidity + severity: ERROR