Skip to content
This repository has been archived by the owner on Feb 24, 2023. It is now read-only.

Add 10 vulnerabilities #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

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


## Credits

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).
Expand Down
19 changes: 19 additions & 0 deletions dangerous_strict_equalities/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
## Dangerous strict equalities

### Description
Use of strict equalities that can be easily manipulated by an attacker.

### Exploit Scenario:

```solidity
contract Crowdsale{
function fund_reached() public returns(bool){
return this.balance == 100 ether;
}
```
`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.

### Mitigations
- Don't use strict equality to determine if an account has enough ethers or tokens.
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue

21 changes: 21 additions & 0 deletions incorrect_erc20_interface/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
## Incorrect erc20 interface

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this makes sense but should there be a generalized rule that all interfaces should be correctly configured to return the correct value? This same type of bug could occur in cases where a developer uses a custom or non ERC-20/721 interface?


### Description
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.

### Exploit Scenario:

```solidity
contract Token{
function transfer(address to, uint value) external;
//...
}
```
`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.

### Recommendation
- Set the appropriate return values and value-types for the defined ERC20 functions.
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue
- Use `slither-check-erc` to ensure ERC's conformance


20 changes: 20 additions & 0 deletions incorrect_erc721_interface/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
## Incorrect erc721 interface

### Description
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.

### Exploit Scenario:

```solidity
contract Token{
function ownerOf(uint256 _tokenId) external view returns (bool);
//...
}
```
`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.

### Mitigations
- Set the appropriate return values and value-types for the defined ERC721 functions.
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue


39 changes: 39 additions & 0 deletions rtlo/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
## Right-To-Left-Override character

### Description
An attacker can manipulate the logic of the contract by using a right-to-left-override character (U+202E)

### Exploit Scenario:

```solidity
contract Token
{

address payable o; // owner
mapping(address => uint) tokens;

function withdraw() external returns(uint)
{
uint amount = tokens[msg.sender];
address payable d = msg.sender;
tokens[msg.sender] = 0;
_withdraw(/*owner‮/*noitanitsed*/ d, o/*‭
/*value */, amount);
}

function _withdraw(address payable fee_receiver, address payable destination, uint value) internal
{
fee_receiver.transfer(1);
destination.transfer(value);
}
}
```

`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.



### Mitigations
- Special control characters must not be allowed.
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue

37 changes: 37 additions & 0 deletions state-variable-shadowing/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
## State variable shadowing

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no link to this document in the main README.md.


### Description
State variables can be shadowed in Solidity.

### Exploit Scenario:

```solidity
contract BaseContract{
address owner;

modifier isOwner(){
require(owner == msg.sender);
_;
}

}

contract DerivedContract is BaseContract{
address owner;

constructor(){
owner = msg.sender;
}

function withdraw() isOwner() external{
msg.sender.transfer(this.balance);
}
}
```
`owner` of `BaseContract` is never assigned and the modifier `isOwner` does not work.

### Mitigations
- Avoid state variable shadowing.
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue


21 changes: 21 additions & 0 deletions suicidal/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
## Suicidal contract

### Description
Unprotected call to a function executing `selfdestruct`/`suicide`.

### Exploit Scenario:

```solidity
contract Suicidal{
function kill() public{
selfdestruct(msg.sender);
}
}
```
Bob calls `kill` and destructs the contract.

### Mitigations
- Protect access to all sensitive functions.
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue


32 changes: 32 additions & 0 deletions tautology/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
## Tautology or contradiction

### Description
Expressions that are tautologies or contradictions.

### Exploit Scenario:

```solidity
contract A {
function f(uint x) public {
// ...
if (x >= 0) { // bad -- always true
// ...
}
// ...
}

function g(uint8 y) public returns (bool) {
// ...
return (y < 512); // bad!
// ...
}
}
```
`x` is an `uint256`, as a result `x >= 0` will be always true.
`y` is an `uint8`, as a result `y <512` will be always true.


### Mitigations
- Avoid tautology
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue

25 changes: 25 additions & 0 deletions uninitialized-state-variables/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
## Uninitialized state variables

### Description
Usage of uninitialized state variables.

### Exploit Scenario:

```solidity
contract Uninitialized{
address destination;

function transfer() payable public{
destination.transfer(msg.value);
}
}
```
Bob calls `transfer`. As a result, the ethers are sent to the address 0x0 and are lost.


### Mitigations
- Initialize all the variables. If a variable is meant to be initialized to zero, explicitly set it to zero.
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue



30 changes: 30 additions & 0 deletions uninitialized-storage/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@

## Uninitialized storage variables

### Description
An uinitialized storage variable will act as a reference to the first state variable, and can override a critical variable.

### Exploit Scenario:

```solidity
contract Uninitialized{
address owner = msg.sender;

struct St{
uint a;
}

function func() {
St st;
st.a = 0x0;
}
}
```
Bob calls `func`. As a result, `owner` is override to 0.


### Mitigations
- Initialize all the storage variables.
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue


23 changes: 23 additions & 0 deletions unused_return/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
## Unused return


### Description
The return value of an external call is not checked.

### Exploit Scenario:

```solidity
contract MyConc{
using SafeMath for uint;
function my_func(uint a, uint b) public{
a.add(b);
}
}
```
`MyConc` calls `add` of SafeMath, but does not store the result in `a`. As a result, the computation has no effect.

### Mitigations
- Ensure that all the return values of the function calls are used.
- Use [Slither](https://github.com/crytic/slither/) or [crytic.io](https://crytic.io/) to detect the issue