Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import not-so-smart-contracts #146

Open
wants to merge 74 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
930ee7c
Initial commit
montyly Aug 28, 2017
0dff584
Add new examples:
montyly Aug 29, 2017
8600067
Minor modifs in Readme(s)
montyly Aug 29, 2017
5f20de6
Improve reentrancy example
montyly Sep 7, 2017
6f1ec23
Make titles consistent
Oct 12, 2017
7f1374e
Add unchecked external call
Oct 12, 2017
1c0e2e3
Add links to lines of code
Oct 12, 2017
8562893
Update README.md
Oct 13, 2017
b9c064d
Update README.md
Oct 13, 2017
1fdd900
small edits
Oct 13, 2017
ae8d419
Added credits and contact info (#5)
ESultanik Oct 13, 2017
0b51260
Add race condition (#3)
montyly Oct 13, 2017
8818252
README nitpics
dguido Oct 15, 2017
c3f5799
correct path
dguido Oct 15, 2017
a47392b
Create LICENSE
dguido Oct 16, 2017
c256330
Clean reentrancy exploit
montyly Mar 6, 2018
2d775c2
Added a README on variable shadowing
ggrieco-tob Mar 19, 2018
0e66892
Added honeypot examples
Apr 2, 2018
93c7242
wrong -> incorrect
dguido Jul 26, 2018
ae26f59
Small spelling correction in file name...
Aug 29, 2018
7928b85
Fix syntax error so solc compiles...
Aug 29, 2018
4397d5e
Denial of service vulns added
blperez01 Aug 31, 2018
d1742f9
Fixed README
blperez01 Aug 31, 2018
7fa9467
Created readme's for two new vulns and updated master readme accordingly
blperez01 Aug 31, 2018
46a7d81
updated bad randomness readme. still need examples
blperez01 Aug 31, 2018
041c3d1
Updated integer overlow and denial of service readme
blperez01 Aug 31, 2018
2cf5365
updated missing constructor and race condition READMEs
blperez01 Aug 31, 2018
a9aaec1
added forced ether reception example
ggrieco-tob Sep 4, 2018
df804f0
More README updates
blperez01 Sep 4, 2018
4196490
Create inherited_state.sol
ggrieco-tob Mar 19, 2018
a2b15da
Another round of readme updates
blperez01 Sep 4, 2018
d214ed6
Fixed links
blperez01 Sep 4, 2018
1afbaea
fixed top level readme, added references
blperez01 Sep 4, 2018
939b0ab
Fix syntax error so solc compiles...
Aug 29, 2018
f619b7f
Update README.md
ggrieco-tob Sep 4, 2018
3c93d0e
More educational clarification for the reentrancy example
ESultanik Sep 5, 2018
0abd0e5
Added forced ether reception to README
blperez01 Sep 5, 2018
d516ef2
Added links, slight format changes
blperez01 Sep 5, 2018
64cfc2a
Fix path to relative link to code
ESultanik Sep 5, 2018
15714f4
Fixed variable shadowing link
blperez01 Sep 5, 2018
2335b71
Update README.md
blperez01 Sep 5, 2018
d075aa8
Update list_dos.sol
blperez01 Sep 19, 2018
5adefba
Update README.md
blperez01 Sep 19, 2018
c46872a
Changed missing constructor to more sensible name
blperez01 Sep 19, 2018
e33f629
removed missing constructor folder
blperez01 Sep 19, 2018
456fa55
Update README.md
blperez01 Sep 19, 2018
7b8fa0b
Fixed the link to the constructor example
Sep 24, 2018
7e72091
Fixed main README link to Wrong Constructor Name
Sep 24, 2018
ea3625d
Added theRun as an example of bad randomness
Sep 27, 2018
c9b9d42
added theRun source code
Sep 27, 2018
56e4af4
updated README to reference the local source code
Sep 27, 2018
d7ac0a7
Add a new reference to unchecked external calls
Oct 3, 2018
6b162c0
Add the same reference to Denial of Service
Oct 4, 2018
778153e
Update README.md
dguido Oct 5, 2018
2966bba
Update README.md
dguido Oct 5, 2018
777bc54
Update README.md
dguido Oct 5, 2018
cba8e6f
updated not-so-smart-contracts to add SpankChain
Oct 9, 2018
d2846df
add more examples
tayvano Feb 25, 2020
77a96d3
Update theRun.sol
May 31, 2022
821d22d
mv everything into subfolder
Oct 3, 2022
00d656c
Merge branch 'master' into import-nssc
Oct 4, 2022
3b41791
Add 10 vulnerabilities
montyly Apr 24, 2020
dac979c
rm nssc/solidity/license
Oct 4, 2022
0d9ada8
describe solc version impact on integer over-/under-flows
Oct 4, 2022
cd1dab9
fix spelling mistakes
Oct 4, 2022
63dbb5c
init solhint config
Oct 4, 2022
e1e63ac
bad_randomness editing pass
Oct 4, 2022
a332427
dos & ether reception editing pass
Oct 4, 2022
4281a02
honeypot editing pass
Oct 4, 2022
897c9ad
remove incorrect interface examples
Oct 4, 2022
953c20a
reentrancy editing pass
Oct 4, 2022
d9e440a
rm old examples already covered by honeypot examples
Oct 4, 2022
edda363
more cleanup & update nssc/solidity readme
Oct 4, 2022
6ca3a11
fix typo
Oct 4, 2022
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
7 changes: 7 additions & 0 deletions .solhint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": "solhint:recommended",
"rules": {
"compiler-version": ["off"],
"func-visibility": ["warn",{ "ignoreConstructors":true }]
}
}
661 changes: 0 additions & 661 deletions LICENSE

This file was deleted.

41 changes: 41 additions & 0 deletions not-so-smart-contracts/solidity/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

# (Not So) Smart Solidity Contracts

This repository contains examples of common Ethereum smart contract vulnerabilities, including code from real smart contracts. Use Not So Smart Contracts to
- learn about EVM and Solidity vulnerabilities
- as a reference when performing security reviews
- and as a benchmark for security and analysis tools.

## Features

Each _Not So Smart Contract_ includes a standard set of information:

* Description of the unique vulnerability type
* Attack scenarios to exploit the vulnerability
* Recommendations to eliminate or mitigate the vulnerability
* Real-world contracts that exhibit the flaw
* References to third-party resources with more information

## Vulnerabilities

| 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 |
| [Honeypots](honeypots) | Adversarial contracts that use obscure edge cases in a wide selection of solidity compiler versions to steal money from you when you think you're taking money from them |
| [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. |
| [Tautology](tautology) | Usage of always boolean expressions that are always true. |
| [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. |


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

If you have questions, problems, or just want to learn more, then join the #ethereum channel on the [Empire Hacking Slack](https://empireslacking.herokuapp.com/) or [contact us](https://www.trailofbits.com/contact/) directly.
30 changes: 30 additions & 0 deletions not-so-smart-contracts/solidity/bad_randomness/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Bad Randomness

Pseudorandom number generation on the blockchain is generally unsafe. There are a number of reasons for this, including:

- The blockchain does not provide any cryptographically secure source of randomness. Block hashes in isolation are cryptographically random, however, a malicious miner can modify block headers, introduce additional transactions, and choose not to publish blocks in order to influence the resulting hashes. Therefore, miner-influenced values like block hashes and timestamps should never be used as a source of randomness.
- Everything in a contract is publicly visible. Random numbers cannot be generated or stored in the contract until after all lottery entries have been stored.
- Computers will always be faster than the blockchain. Any number that the contract could generate can potentially be precalculated off-chain before the end of the block.

A common workaround for the lack of on-chain randomness is using a commit and reveal scheme. Here, each user submits the hash of their secret number. When the time comes for the random number to be generated, each user sends their secret number to the contract which then verifies it matches the hash submitted earlier and XORs them together. Therefore no participant can observe how their contribution will affect the end result until after everyone has already committed to a value. However, this is also vulnerable to DoS attacks, since the last person to reveal can choose to never submit their secret. Even if the contract is allowed to move forward without everyone's secrets, this gives them influence over the end result. In general, we do not recommend commit and reveal schemes.

## Attack Scenarios

- A lottery where people bet on whether the hash of the current block is even or odd. A miner that bets on even can throw out blocks whose hash are odd.
- A commit-reveal scheme where users don't necessarily have to reveal their secret (to prevent DoS). A user has money riding on the outcome of the PRNG and submits a large number of commits, allowing them to choose the one they want to reveal at the end.

## Mitigations

There are not currently any guaranteed solutions for this issue. Do not build applications that require on-chain randomness. In the future, however, these approaches show promise

- [Verifiable delay functions](https://eprint.iacr.org/2018/601.pdf): functions which produce a pseudorandom number and take a fixed amount of sequential time to evaluate
- [RANDAO](https://github.com/randao/randao): A commit reveal scheme where users must stake wei to participate

## Examples

- The `random` function in [TheRun](TheRun.sol) is vulnerable to this attack. It uses the blockhash, timestamp and block number to generate numbers in a range to determine winners of the lottery. To exploit this, an attacker could set up a smart contract that generates numbers in the same way and submits entries when it would win. As well, the miner of the block has some control over the blockhash and timestamp and would also be able to influence the lottery in their favor.

## Sources

- [StackExchange](https://ethereum.stackexchange.com/questions/191/how-can-i-securely-generate-a-random-number-in-my-smart-contract)
- [Blog Post](https://blog.positive.com/predicting-random-numbers-in-ethereum-smart-contracts-e5358c6b8620)
166 changes: 166 additions & 0 deletions not-so-smart-contracts/solidity/bad_randomness/theRun.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
pragma solidity ^0.8.17;

contract TheRun {
// solhint-disable-next-line not-rely-on-time
uint256 constant private SALT = block.timestamp;

address private admin;
uint private balance = 0;
uint private payoutId = 0;
uint private lastPayout = 0;
uint private winningPot = 0;
uint private minMultiplier = 1100; //110%

//Fees are necessary and set very low, the fees will decrease each time they are collected.
//Fees are just here to maintain the website at beginning, and will progressively go to 0% :)
uint private fees = 0;
uint private feeFrac = 20; //Fraction for fees in per"thousand", not percent, so 20 is 2%

uint private potFrac = 30; //For the winningPot ,30=> 3% are collected. This is fixed.

constructor() {
admin = msg.sender;
}

modifier onlyowner {if (msg.sender == admin) _; }

struct Player {
address addr;
uint payout;
bool paid;
}

Player[] private players;

//--Fallback function
fallback() payable {
init();
}

//--initiated function
function init() private {
uint deposit = msg.value;
if (msg.value < 500 finney) { // only participation with >1 ether accepted
msg.sender.transfer(msg.value);
return;
}
if (msg.value > 20 ether) { //only participation with <20 ether accepted
msg.sender.transfer(msg.value - (20 ether));
deposit=20 ether;
}
participate(deposit);
}

//------- Core of the game----------
function participate(uint deposit) private {
//calculate the multiplier to apply to the future payout

uint totalMultiplier = minMultiplier; //initiate totalMultiplier
if (balance < 1 ether && players.length > 1) {
totalMultiplier += 100; // + 10 %
}
if ((players.length % 10) == 0 && players.length > 1) { //Every 10th participant gets a 10% bonus
totalMultiplier += 100; // + 10 %
}

//add new player in the queue !
players.push(Player(msg.sender, (deposit * totalMultiplier) / 1000, false));

//--- UPDATING CONTRACT STATS ----
winningPot += (deposit * potFrac) / 1000; // take some 3% to add for the winning pot !
fees += (deposit * feeFrac) / 1000; // collect maintenance fees 2%
balance += (deposit * (1000 - ( feeFrac + potFrac ))) / 1000; // update balance

//Classic payout for the participants
while (balance > players[payoutId].payout) {
lastPayout = players[payoutId].payout;
balance -= players[payoutId].payout; // update the balance
players[payoutId].paid=true;
players[payoutId].addr.transfer(lastPayout); // pay the man
// solhint-disable-next-line reentrancy
payoutId += 1;
}

// Winning the Pot :) Condition : paying at least 1 people with deposit > 2 ether and having luck !
if (( deposit > 1 ether ) && (deposit > players[payoutId].payout)) {
uint roll = random(100); // take a random number between 1 & 100
if (roll % 10 == 0 ) { // if lucky : Chances : 1 out of 10 !
// solhint-disable-next-line reentrancy
winningPot = 0;
msg.sender.transfer(winningPot); // Bravo !
}
}

}

function random(uint max) private constant returns (uint256 result) {
//get the best seed for randomness
uint256 x = SALT * 100 / max;
uint256 y = SALT * block.number / (SALT % 5) ;
uint256 seed = block.number/3 + (SALT % 300) + lastPayout +y;
// solhint-disable-next-line not-rely-on-block-hash
uint256 h = uint256(block.blockhash(seed));
return uint256((h / x)) % max + 1; //random number between 1 and max
}

//---Contract management functions
function changeOwnership(address _owner) external onlyowner {
admin = _owner;
}
function watchBalance() external constant returns(uint totalBalance) {
totalBalance = balance / 1 wei;
}

function watchBalanceInEther() external constant returns(uint totalBalanceInEther) {
totalBalanceInEther = balance / 1 ether;
}

//Fee functions for creator
function collectAllFees() external onlyowner {
require(fees == 0, "No fees to collect");
feeFrac -= 1;
fees = 0;
admin.transfer(fees);
}

function getAndReduceFeesByFraction(uint p) external onlyowner {
if (fees == 0) feeFrac -= 1; // reduce fees.
fees -= fees / 1000 * p;
admin.transfer(fees / 1000 * p); // send a percent of fees
}


//---Contract informations
function nextPayout() external constant returns(uint next) {
next = players[payoutId].payout / 1 wei;
}

function watchFees() external constant returns(uint collectedFees) {
collectedFees = fees / 1 wei;
}

function watchWinningPot() external constant returns(uint winningPot) {
winningPot = winningPot / 1 wei;
}

function watchLastPayout() external constant returns(uint payout) {
payout = lastPayout;
}

function totalOfPlayers() external constant returns(uint numberOfPlayers) {
numberOfPlayers = players.length;
}

function playerInfo(uint id) external constant returns(address player, uint payout, bool userPaid) {
if (id <= players.length) {
player = players[id].addr;
payout = players[id].payout / 1 wei;
userPaid=players[id].paid;
}
}

function payoutQueueSize() external constant returns(uint queueSize) {
queueSize = players.length - payoutId;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
## 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. If `Crowdsale` reaches 100 ether and Bob sends 0.1 ether, `fund_reached` is always false and the crowdsale would never end.

### Mitigations

- Don't use strict equality to determine if an account has sufficient ethers or tokens.
- Use [slither](https://github.com/crytic/slither/) to detect this issue.
20 changes: 20 additions & 0 deletions not-so-smart-contracts/solidity/denial_of_service/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Denial of Service

A malicious contract can permanently stall another contract that calls it by failing in a strategic way. In particular, contracts that bulk perform transactions or updates using a `for` loop can be DoS'd if a call to another contract or `transfer` fails during the loop.

## Attack Scenarios

- Auction contract where the previous winner must be reimbursed when they are outbid. If the call refunding the previous winner continuously fails, the auction is stalled and they become the de facto winner. It's better to use a pull-pattern that flags funds as eligible for withdrawal. See examples of an [insecure](auction.sol#L4) and [secure](auction#L24) version of this auction pattern.
- Contract iterates through an array to pay back its users. If one `transfer` fails in the middle of a `for` loop all reimbursements fail. See [this insecure example](list_dos.sol#L3) for an example of doing this wrong.
- Attacker spams contract, causing some array to become large. Then `for` loops iterating through the array might run out of gas and revert. See [this example](list_dos.sol#L26) that pauses & results list processing to prevent getting stuck due to out-of-gas errors.

## Mitigations

- Favor the pull-pattern: make funds available for users to withdraw rather than trying to send funds to users.
- If iterating over a dynamically sized data structure, be able to handle the case where the function takes multiple blocks to execute. One strategy for this is storing an iterator in a private variable and using `while` loop that stops when gas drops below certain threshold.

## References

- [Reddit conversation about stuck contract](https://www.reddit.com/r/ethereum/comments/4ghzhv/governmentals_1100_eth_jackpot_payout_is_stuck/)
- [ConsenSys re unexpected reverts](https://github.com/ConsenSys/smart-contract-best-practices#dos-with-unexpected-revert)
- [Griefing wallets](https://vessenes.com/ethereum-griefing-wallets-send-w-throw-considered-harmful/)
49 changes: 49 additions & 0 deletions not-so-smart-contracts/solidity/denial_of_service/auction.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
pragma solidity ^0.8.17;

// Auction susceptible to DoS attack
contract InsecureAuction {
address public currentWinner = address(0);
uint public currentBid = 0;

// Takes in bid, refunding the previous winner if they are outbid
function bid() public payable {
require(msg.value > currentBid, "Too little value to bid");
// If the refund fails, the entire transaction reverts.
// Therefore a bidder who always fails will win
// E.g. if recipients fallback function is just revert()
if (currentWinner != 0) {
require(currentWinner.send(currentBid), "Send failure");
}
currentWinner = msg.sender; // solhint-disable-line reentrancy
currentBid = msg.value; // solhint-disable-line reentrancy
}

}

// Auction that is NOT susceptible to DoS attack
contract SecureAuction {
address public currentWinner;
uint public currentBid;

// Store refunds in mapping to avoid DoS
mapping(address => uint) public refunds;

// Avoids "pushing" balance to users favoring "pull" architecture
function bid() external payable {
require(msg.value > currentBid, "Too little value to bid");
if (currentWinner != 0) {
refunds[currentWinner] += currentBid;
}
currentWinner = msg.sender;
currentBid = msg.value;
}

// Allows users to get their refund from auction
function withdraw() public {
// Do all state manipulation before external call to avoid reentrancy attack
uint refund = refunds[msg.sender];
refunds[msg.sender] = 0;
msg.sender.transfer(refund); // even if this reverts, calls to bid() can still succeed
}

}
29 changes: 29 additions & 0 deletions not-so-smart-contracts/solidity/denial_of_service/list_dos.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
pragma solidity ^0.8.17;

contract CrowdFundBad {
address[] private refundAddresses;
mapping(address => uint) public refundAmount;
function badRefund() public {
for(uint i; i < refundAddresses.length; i++) {
// If one of the following transfers reverts, they all revert
refundAddresses[i].transfer(refundAmount[refundAddresses[i]]);
}
}
}

// This is safe against the list length causing out of gas issues
// This is NOT safe against the payee causing the execution to revert
contract CrowdFundSafer {
address[] private refundAddresses;
mapping(address => uint) public refundAmount;
uint256 public nextIdx;
function refundSafe() public {
uint256 i = nextIdx;
// Refunds are only processed as long as sufficient gas remains
while(i < refundAddresses.length && msg.gas > 200000) {
refundAddresses[i].transfer(refundAmount[i]);
i++;
}
nextIdx = i; // solhint-disable-line reentrancy
}
}
Loading