Skip to content

Commit 29f8165

Browse files
checkpoint: add Vault 01 and 02
Signed-off-by: Elliot <elliotfriedman3@gmail.com>
1 parent 14ee409 commit 29f8165

File tree

13 files changed

+553
-24
lines changed

13 files changed

+553
-24
lines changed

README.md

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,12 @@ This workshop will walk you through the basics of securing smart contracts by te
77
In this workshop we will be protecting a governance heavy application from the consequences of malicious upgrades and or deployment scripts.
88

99
## Bug Types
10-
- **Deployment** - Incorrect parameters, caught with a validation
11-
- **Deployment** - Incorrect parameters, caught with an integration test
10+
- **Deployment** - Incorrect parameters, caught with a validation
11+
- **Deployment** - Incorrect parameters, caught with an integration test
1212
****
1313
- **Upgrade** - Storage offset changes, caught with an integration test, also caught with a validation
1414
- **Upgrade** - Logic error, caught with an integration test
1515
****
16-
- **New contract** - Logic error, caught with formal verification
17-
- **New contract** - Vulnerable to DoS vector, caught with static analysis
18-
- **New contract** - Vulnerable to arbitrary calldata and targets, caught with static analysis
1916

2017
### Further Reading
2118

src/examples/00/REQUIREMENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Overview
22

3-
The business team needs a simple contract that allows swapping between various different tokens of the same value. This is a peg stability module that allows users to deposit one asset, and withdraw another of the same value. It does not use chainlink and assumes price parity of all assets. There are no swap fees, and LP deposit then withdraw to swap.
3+
The business team needs a simple contract that allows swapping between various different tokens of the same value. This is a peg stability module that allows users to deposit one asset, and withdraw another of the same value. It does not use chainlink and assumes price parity of all assets. There are no swap fees, and LP's deposit then withdraw to swap.
44

55
Create a contract that allows users to deposit any of the supported tokens, and withdraw any of the supported tokens.
66

File renamed without changes.

src/examples/01/REQUIREMENTS.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Overview
2+
3+
The first round of security reviews was conducted by an external security firm. The results were pretty bad for such a simple contract, and the CTO is livid. You show up to the meeting and get yelled at for writing such bad code. The CTO is demanding you fix the code immediately. They refused to give you the findings, so you're on your own.
4+
5+
Good luck.

src/examples/01/TESTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# Vault Test Cases
2+
3+
- decimal scaling logic is correct
4+
- multiple users deposit, each with different tokens, and withdraw different tokens than deposited

src/examples/01/Vault01.sol

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
pragma solidity 0.8.25;
2+
3+
import {IERC20Metadata} from
4+
"@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
5+
import {SafeERC20} from
6+
"@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
7+
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
8+
9+
contract Vault {
10+
using SafeERC20 for IERC20;
11+
12+
/// @notice Mapping of authorized tokens
13+
mapping(address => bool) public authorizedToken;
14+
15+
/// @notice User's balance of all tokens deposited in the vault
16+
mapping(address => uint256) public balanceOf;
17+
18+
/// @notice Total amount of tokens supplied to the vault
19+
///
20+
/// invariants:
21+
/// totalSupplied = sum(balanceOf all users)
22+
/// sum(balanceOf(vault) authorized tokens) >= totalSupplied
23+
///
24+
uint256 public totalSupplied;
25+
26+
/// @notice Deposit event
27+
/// @param token The token deposited
28+
/// @param sender The address that deposited the token
29+
/// @param amount The amount deposited
30+
event Deposit(
31+
address indexed token, address indexed sender, uint256 amount
32+
);
33+
34+
/// @notice Withdraw event
35+
/// @param token The token withdrawn
36+
/// @param sender The address that withdrew the token
37+
/// @param amount The amount withdrawn
38+
event Withdraw(
39+
address indexed token, address indexed sender, uint256 amount
40+
);
41+
42+
/// @notice Construct the vault with a list of authorized tokens
43+
/// @param _tokens The list of authorized tokens
44+
constructor(address[] memory _tokens) {
45+
for (uint256 i = 0; i < _tokens.length; i++) {
46+
require(
47+
IERC20Metadata(_tokens[i]).decimals() <= 18,
48+
"unsupported decimals"
49+
);
50+
authorizedToken[_tokens[i]] = true;
51+
}
52+
}
53+
54+
/// @notice Deposit tokens into the vault
55+
/// @param token The token to deposit, only authorized tokens allowed
56+
/// @param amount The amount to deposit
57+
function deposit(address token, uint256 amount) external {
58+
require(authorizedToken[token], "Vault: token not authorized");
59+
60+
uint256 normalizedAmount = getNormalizedAmount(token, amount);
61+
62+
/// save on gas by using unchecked, no need to check for overflow
63+
/// as all deposited tokens are whitelisted
64+
unchecked {
65+
balanceOf[msg.sender] += normalizedAmount;
66+
}
67+
68+
totalSupplied += normalizedAmount;
69+
70+
IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
71+
72+
emit Deposit(token, msg.sender, amount);
73+
}
74+
75+
/// @notice Withdraw tokens from the vault
76+
/// @param token The token to withdraw, only authorized tokens are allowed
77+
/// this is implicitly checked because a user can only have a balance of an
78+
/// authorized token
79+
/// @param amount The amount to withdraw
80+
function withdraw(address token, uint256 amount) external {
81+
require(authorizedToken[token], "Vault: token not authorized");
82+
83+
uint256 normalizedAmount = getNormalizedAmount(token, amount);
84+
85+
/// both a check and an effect, ensures user has sufficient funds for
86+
/// withdrawal
87+
/// must be checked for underflow as a user can only withdraw what they
88+
/// have deposited
89+
balanceOf[msg.sender] -= normalizedAmount;
90+
91+
/// save on gas by using unchecked, no need to check for underflow
92+
/// as all deposited tokens are whitelisted, plus we know our invariant
93+
/// always holds
94+
unchecked {
95+
totalSupplied -= normalizedAmount;
96+
}
97+
98+
IERC20(token).safeTransfer(msg.sender, amount);
99+
100+
emit Withdraw(token, msg.sender, amount);
101+
}
102+
103+
/// @notice public for testing purposes, returns the normalized amount of
104+
/// tokens scaled to 18 decimals
105+
/// @param token The token to deposit
106+
/// @param amount The amount to deposit
107+
function getNormalizedAmount(address token, uint256 amount)
108+
public
109+
view
110+
returns (uint256 normalizedAmount)
111+
{
112+
uint8 decimals = IERC20Metadata(token).decimals();
113+
if (decimals < 18) {
114+
/// scale the amount to 18 decimals
115+
/// unchecked because we know that the product will always be less
116+
/// than 2^256-1 as 1e18 = $1
117+
unchecked {
118+
normalizedAmount = amount * (10 ** (18 - decimals));
119+
}
120+
} else if (decimals > 18) {
121+
revert("Vault: unsupported decimals over 18");
122+
}
123+
}
124+
}

src/examples/02/REQUIREMENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# Overview
2+
3+
New requirements have been added.
4+
5+
The contract needs a way for an owner to add and remove tokens from the whitelisted tokens list. You counter and say that the complexity of adding removal logic would greatly increase the attack surface of the contract. The business team agrees to cut the removal logic, but they still want to be able to add tokens to the whitelist after deployment.
6+

src/examples/02/TESTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# Vault Test Cases
2+
3+
- decimal scaling logic is correct
4+
- multiple users deposit, each with different tokens, and withdraw different tokens than deposited

src/examples/02/Vault02.sol

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
pragma solidity 0.8.25;
2+
3+
import {IERC20Metadata} from
4+
"@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
5+
import {SafeERC20} from
6+
"@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
7+
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
8+
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
9+
10+
contract Vault is Ownable {
11+
using SafeERC20 for IERC20;
12+
13+
/// @notice Mapping of authorized tokens
14+
mapping(address => bool) public authorizedToken;
15+
16+
/// @notice User's balance of all tokens deposited in the vault
17+
mapping(address => uint256) public balanceOf;
18+
19+
/// @notice Total amount of tokens supplied to the vault
20+
///
21+
/// invariants:
22+
/// totalSupplied = sum(balanceOf all users)
23+
/// sum(balanceOf(vault) authorized tokens) >= totalSupplied
24+
///
25+
uint256 public totalSupplied;
26+
27+
/// @notice Deposit event
28+
/// @param token The token deposited
29+
/// @param sender The address that deposited the token
30+
/// @param amount The amount deposited
31+
event Deposit(
32+
address indexed token, address indexed sender, uint256 amount
33+
);
34+
35+
/// @notice Withdraw event
36+
/// @param token The token withdrawn
37+
/// @param sender The address that withdrew the token
38+
/// @param amount The amount withdrawn
39+
event Withdraw(
40+
address indexed token, address indexed sender, uint256 amount
41+
);
42+
43+
event TokenAdded(address indexed token);
44+
45+
/// @notice Construct the vault with a list of authorized tokens
46+
/// @param _tokens The list of authorized tokens
47+
constructor(address[] memory _tokens, address _owner) Ownable(_owner) {
48+
for (uint256 i = 0; i < _tokens.length; i++) {
49+
require(
50+
IERC20Metadata(_tokens[i]).decimals() <= 18,
51+
"Vault: unsupported decimals"
52+
);
53+
54+
authorizedToken[_tokens[i]] = true;
55+
56+
emit TokenAdded(_tokens[i]);
57+
}
58+
}
59+
60+
/// -------------------------------------------------------------
61+
/// -------------------------------------------------------------
62+
/// -------------------- ONLY OWNER FUNCTION --------------------
63+
/// -------------------------------------------------------------
64+
/// -------------------------------------------------------------
65+
66+
/// @notice Add a token to the list of authorized tokens
67+
/// only callable by the owner
68+
/// @param token to add
69+
function addToken(address token) external onlyOwner {
70+
require(
71+
IERC20Metadata(token).decimals() <= 18,
72+
"Vault: unsupported decimals"
73+
);
74+
require(!authorizedToken[token], "Vault: token already authorized");
75+
76+
authorizedToken[token] = true;
77+
78+
emit TokenAdded(token);
79+
}
80+
81+
/// -------------------------------------------------------------
82+
/// -------------------------------------------------------------
83+
/// ----------------- PUBLIC MUTATIVE FUNCTIONS -----------------
84+
/// -------------------------------------------------------------
85+
/// -------------------------------------------------------------
86+
87+
/// @notice Deposit tokens into the vault
88+
/// @param token The token to deposit, only authorized tokens allowed
89+
/// @param amount The amount to deposit
90+
function deposit(address token, uint256 amount) external {
91+
require(authorizedToken[token], "Vault: token not authorized");
92+
93+
uint256 normalizedAmount = getNormalizedAmount(token, amount);
94+
95+
/// save on gas by using unchecked, no need to check for overflow
96+
/// as all deposited tokens are whitelisted
97+
unchecked {
98+
balanceOf[msg.sender] += normalizedAmount;
99+
}
100+
101+
totalSupplied += normalizedAmount;
102+
103+
IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
104+
105+
emit Deposit(token, msg.sender, amount);
106+
}
107+
108+
/// @notice Withdraw tokens from the vault
109+
/// @param token The token to withdraw, only authorized tokens are allowed
110+
/// this is implicitly checked because a user can only have a balance of an
111+
/// authorized token
112+
/// @param amount The amount to withdraw
113+
function withdraw(address token, uint256 amount) external {
114+
require(authorizedToken[token], "Vault: token not authorized");
115+
116+
uint256 normalizedAmount = getNormalizedAmount(token, amount);
117+
118+
/// both a check and an effect, ensures user has sufficient funds for
119+
/// withdrawal
120+
/// must be checked for underflow as a user can only withdraw what they
121+
/// have deposited
122+
balanceOf[msg.sender] -= normalizedAmount;
123+
124+
/// save on gas by using unchecked, no need to check for underflow
125+
/// as all deposited tokens are whitelisted, plus we know our invariant
126+
/// always holds
127+
unchecked {
128+
totalSupplied -= normalizedAmount;
129+
}
130+
131+
IERC20(token).safeTransfer(msg.sender, amount);
132+
133+
emit Withdraw(token, msg.sender, amount);
134+
}
135+
136+
/// --------------------------------------------------------
137+
/// --------------------------------------------------------
138+
/// ----------------- PUBLIC VIEW FUNCTION -----------------
139+
/// --------------------------------------------------------
140+
/// --------------------------------------------------------
141+
142+
/// @notice public for testing purposes, returns the normalized amount of
143+
/// tokens scaled to 18 decimals
144+
/// @param token The token to deposit
145+
/// @param amount The amount to deposit
146+
function getNormalizedAmount(address token, uint256 amount)
147+
public
148+
view
149+
returns (uint256 normalizedAmount)
150+
{
151+
uint8 decimals = IERC20Metadata(token).decimals();
152+
normalizedAmount = amount;
153+
if (decimals < 18) {
154+
normalizedAmount = amount * (10 ** (18 - decimals));
155+
}
156+
}
157+
}

src/proposals/sips/SIP00.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {GovernorBravoProposal} from
55
"@forge-proposal-simulator/src/proposals/GovernorBravoProposal.sol";
66
import {Addresses} from "@forge-proposal-simulator/addresses/Addresses.sol";
77

8-
import {Vault} from "src/examples/00/Vault.sol";
8+
import {Vault} from "src/examples/00/Vault00.sol";
99
import {MockToken} from "@mocks/MockToken.sol";
1010
import {ForkSelector, ETHEREUM_FORK_ID} from "@test/utils/Forks.sol";
1111

0 commit comments

Comments
 (0)