Fix: arbitrary-low-level-call matching workarounds
plotchy opened this issue · 1 comments
plotchy commented
Great repo. I've been learning semgrep with your examples and believe I've found a workaround to incorporate arbitrary-low-level-calls without waiting for beta improvements to solidity semgrep.
The fixes include:
- Defining types for metavariables inside function declarations
- Adding pattern-either patterns to include curly braces for
call
rules:
-
id: arbitrary-low-level-call
message: An attacker may perform call() to an arbitrary address with controlled calldata
metadata:
references:
- https://twitter.com/CertiKAlert/status/1512198846343954445
- https://twitter.com/SlowMist_Team/status/1508787862791069700
- https://twitter.com/Beosin_com/status/1509099103401127942
- https://blocksecteam.medium.com/li-fi-attack-a-cross-chain-bridge-vulnerability-no-its-due-to-unchecked-external-call-c31e7dadf60f
- https://etherscan.io/address/0xe7597f774fd0a15a617894dc39d45a28b97afa4f # Auctus Options
- https://etherscan.io/address/0x73a499e043b03fc047189ab1ba72eb595ff1fc8e # Li.Fi
category: controlled-call
tags:
- auctus-options
- starstream-finance
- basket-dao
- li-finance
patterns:
- pattern-either:
- pattern-inside: |
function $F(..., address $ADDR, ..., bytes calldata $DATA, ...) external { ... }
- pattern-inside: |
function $F(..., address $ADDR, ..., bytes calldata $DATA, ...) public { ... }
- pattern-either:
- pattern: $ADDR.call($DATA);
- pattern: $ADDR.call{$VALUE:...}($DATA);
- pattern: $ADDR.call{$VALUE:..., $GAS:...}($DATA);
languages:
- solidity
severity: ERROR
Four matching cases:
contract wxBTRFLY is FrozenToken {
function execute(
address to,
uint256 value,
bytes calldata data
) external returns (bool, bytes memory) {
// ruleid: arbitrary-low-level-call
(bool success, bytes memory result) = to.call{value: value}(data);
// ruleid: arbitrary-low-level-call
(bool success, bytes memory result) = to.call{gas: value}(data);
// ruleid: arbitrary-low-level-call
(bool success, bytes memory result) = to.call(data);
// ruleid: arbitrary-low-level-call
(bool success, bytes memory result) = to.call{value: value, gas: 0}(data);
return (success, result);
}
}
Raz0r commented
Hey @plotchy, thank you for this info. Fixed one more rule by adding argument types. BTW a new semgrep has been released with a better Solidity parser (https://github.com/returntocorp/semgrep/releases/tag/v0.100.0), will test remaining unfixed rules with it.