Skip to content

Commit 3b41791

Browse files
montylybohendo
authored andcommitted
Add 10 vulnerabilities
1 parent 00d656c commit 3b41791

File tree

11 files changed

+277
-0
lines changed

11 files changed

+277
-0
lines changed

not-so-smart-contracts/solidity/README.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,27 @@ Bonus! We have also included a repository and analysis of several [honeypots](ho
1919
| Not So Smart Contract | Description |
2020
| --- | --- |
2121
| [Bad randomness](bad_randomness) | Contract attempts to get on-chain randomness, which can be manipulated by users |
22+
| [Dangerous Strict Equalities](dangerous_strict_equalities) | Use of strict equalities that can be easily manipulated by an attacker. |
2223
| [Denial of Service](denial_of_service) | Attacker stalls contract execution by failing in strategic way |
2324
| [Forced Ether Reception](forced_ether_reception) | Contracts can be forced to receive Ether |
25+
| [Incorrect ERC20 Interface](incorrect_erc20_interface) | Token not implementing the ERC20 interface correctly. |
26+
| [Incorrect ERC721 Interface](incorrect_erc721_interface) | Token not implementing the ERC721 interface correctly. |
2427
| [Incorrect Interface](incorrect_interface) | Implementation uses different function signatures than interface |
2528
| [Integer Overflow](integer_overflow) | Arithmetic in Solidity (or EVM) is not safe by default |
2629
| [Race Condition](race_condition) | Transactions can be frontrun on the blockchain |
2730
| [Reentrancy](reentrancy) | Calling external contracts gives them control over execution |
31+
| [rtlo](rtlo) | Usage of malicious unicode character. |
32+
| [Suicidal](suicidal) | Contract that can be destructed by anyone. |
33+
| [Tautology](tautology) | Usage of always boolean expressions that are always true. |
2834
| [Unchecked External Call](unchecked_external_call) | Some Solidity operations silently fail |
35+
| [Uninitialized State Variables](uninitialized-state-variables) | State variables that are used before being initialized. |
36+
| [Uninitialized Storage Variables](uninitialized-storage-variables) | Storage variables that are used before being initialized. |
2937
| [Unprotected Function](unprotected_function) | Failure to use function modifier allows attacker to manipulate contract |
38+
| [Unused Return Value ](unused-return) | Return values from calls that is not used. |
3039
| [Variable Shadowing](variable%20shadowing/) | Local variable name is identical to one in outer scope |
3140
| [Wrong Constructor Name](wrong_constructor_name) | Anyone can become owner of contract due to missing constructor |
3241

42+
3343
## Credits
3444

3545
These examples are developed and maintained by [Trail of Bits](https://www.trailofbits.com/). Contributions are encouraged and are covered under our [bounty program](https://github.com/trailofbits/not-so-smart-contracts/wiki#bounties).
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
## Dangerous strict equalities
2+
3+
### Description
4+
Use of strict equalities that can be easily manipulated by an attacker.
5+
6+
### Exploit Scenario:
7+
8+
```solidity
9+
contract Crowdsale{
10+
function fund_reached() public returns(bool){
11+
return this.balance == 100 ether;
12+
}
13+
```
14+
`Crowdsale` relies on `fund_reached` to know when to stop the sale of tokens. `Crowdsale` reaches 100 ether. Bob sends 0.1 ether. As a result, `fund_reached` is always false and the crowdsale never ends.
15+
16+
### Mitigations
17+
- Don't use strict equality to determine if an account has enough ethers or tokens.
18+
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue
19+
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
## Incorrect erc20 interface
2+
3+
### Description
4+
Incorrect return values for ERC20 functions. A contract compiled with solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing.
5+
6+
### Exploit Scenario:
7+
8+
```solidity
9+
contract Token{
10+
function transfer(address to, uint value) external;
11+
//...
12+
}
13+
```
14+
`Token.transfer` does not return a boolean. Bob deploys the token. Alice creates a contract that interacts with it but assumes a correct ERC20 interface implementation. Alice's contract is unable to interact with Bob's contract.
15+
16+
### Recommendation
17+
- Set the appropriate return values and value-types for the defined ERC20 functions.
18+
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue
19+
- Use `slither-check-erc` to ensure ERC's conformance
20+
21+
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
## Incorrect erc721 interface
2+
3+
### Description
4+
Incorrect return values for ERC721 functions. A contract compiled with solidity > 0.4.22 interacting with these functions will fail to execute them, as the return value is missing.
5+
6+
### Exploit Scenario:
7+
8+
```solidity
9+
contract Token{
10+
function ownerOf(uint256 _tokenId) external view returns (bool);
11+
//...
12+
}
13+
```
14+
`Token.ownerOf` does not return an address as ERC721 expects. Bob deploys the token. Alice creates a contract that interacts with it but assumes a correct ERC721 interface implementation. Alice's contract is unable to interact with Bob's contract.
15+
16+
### Mitigations
17+
- Set the appropriate return values and value-types for the defined ERC721 functions.
18+
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue
19+
20+
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
## Right-To-Left-Override character
2+
3+
### Description
4+
An attacker can manipulate the logic of the contract by using a right-to-left-override character (U+202E)
5+
6+
### Exploit Scenario:
7+
8+
```solidity
9+
contract Token
10+
{
11+
12+
address payable o; // owner
13+
mapping(address => uint) tokens;
14+
15+
function withdraw() external returns(uint)
16+
{
17+
uint amount = tokens[msg.sender];
18+
address payable d = msg.sender;
19+
tokens[msg.sender] = 0;
20+
_withdraw(/*owner‮/*noitanitsed*/ d, o/*‭
21+
/*value */, amount);
22+
}
23+
24+
function _withdraw(address payable fee_receiver, address payable destination, uint value) internal
25+
{
26+
fee_receiver.transfer(1);
27+
destination.transfer(value);
28+
}
29+
}
30+
```
31+
32+
`Token` uses the right-to-left-override character when calling `_withdraw`. As a result, the fee is incorrectly sent to `msg.sender`, and the token balance is sent to the owner.
33+
34+
35+
36+
### Mitigations
37+
- Special control characters must not be allowed.
38+
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue
39+
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
## State variable shadowing
2+
3+
### Description
4+
State variables can be shadowed in Solidity.
5+
6+
### Exploit Scenario:
7+
8+
```solidity
9+
contract BaseContract{
10+
address owner;
11+
12+
modifier isOwner(){
13+
require(owner == msg.sender);
14+
_;
15+
}
16+
17+
}
18+
19+
contract DerivedContract is BaseContract{
20+
address owner;
21+
22+
constructor(){
23+
owner = msg.sender;
24+
}
25+
26+
function withdraw() isOwner() external{
27+
msg.sender.transfer(this.balance);
28+
}
29+
}
30+
```
31+
`owner` of `BaseContract` is never assigned and the modifier `isOwner` does not work.
32+
33+
### Mitigations
34+
- Avoid state variable shadowing.
35+
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue
36+
37+
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
## Suicidal contract
2+
3+
### Description
4+
Unprotected call to a function executing `selfdestruct`/`suicide`.
5+
6+
### Exploit Scenario:
7+
8+
```solidity
9+
contract Suicidal{
10+
function kill() public{
11+
selfdestruct(msg.sender);
12+
}
13+
}
14+
```
15+
Bob calls `kill` and destructs the contract.
16+
17+
### Mitigations
18+
- Protect access to all sensitive functions.
19+
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue
20+
21+
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
## Tautology or contradiction
2+
3+
### Description
4+
Expressions that are tautologies or contradictions.
5+
6+
### Exploit Scenario:
7+
8+
```solidity
9+
contract A {
10+
function f(uint x) public {
11+
// ...
12+
if (x >= 0) { // bad -- always true
13+
// ...
14+
}
15+
// ...
16+
}
17+
18+
function g(uint8 y) public returns (bool) {
19+
// ...
20+
return (y < 512); // bad!
21+
// ...
22+
}
23+
}
24+
```
25+
`x` is an `uint256`, as a result `x >= 0` will be always true.
26+
`y` is an `uint8`, as a result `y <512` will be always true.
27+
28+
29+
### Mitigations
30+
- Avoid tautology
31+
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue
32+
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
## Uninitialized state variables
2+
3+
### Description
4+
Usage of uninitialized state variables.
5+
6+
### Exploit Scenario:
7+
8+
```solidity
9+
contract Uninitialized{
10+
address destination;
11+
12+
function transfer() payable public{
13+
destination.transfer(msg.value);
14+
}
15+
}
16+
```
17+
Bob calls `transfer`. As a result, the ethers are sent to the address 0x0 and are lost.
18+
19+
20+
### Mitigations
21+
- Initialize all the variables. If a variable is meant to be initialized to zero, explicitly set it to zero.
22+
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue
23+
24+
25+
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
2+
## Uninitialized storage variables
3+
4+
### Description
5+
An uinitialized storage variable will act as a reference to the first state variable, and can override a critical variable.
6+
7+
### Exploit Scenario:
8+
9+
```solidity
10+
contract Uninitialized{
11+
address owner = msg.sender;
12+
13+
struct St{
14+
uint a;
15+
}
16+
17+
function func() {
18+
St st;
19+
st.a = 0x0;
20+
}
21+
}
22+
```
23+
Bob calls `func`. As a result, `owner` is override to 0.
24+
25+
26+
### Mitigations
27+
- Initialize all the storage variables.
28+
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue
29+
30+

0 commit comments

Comments
 (0)