RACE #21 Of The Secureum Bootcamp Epoch∞
This is a Write-Up of RACE-21, Quiz of the Secureum Bootcamp (opens in a new tab) for Ethereum Smart Contract Auditors. Secureum Mentor Noah (opens in a new tab), one of my colleagues at Spearbit DAO.
Participants of this quiz had to answer 8 questions within the strict time limit of 16 minutes (yes, the time limit includes reading the code). If you’re reading this in preparation for participating yourself, it’s best to give it a try under the same time limit!
As usual, I waited for submissions to close before publishing it and, to stay true to the original, I omitted syntax highlighting. Feel free to copy it into your favorite editor, but do so after starting the timer!
September 9, 2023 by patrickd
Code
All 8 questions in this RACE are based on the below contracts. This is the same contracts you will see for all the 8 questions in this RACE. The question is below the shown contracts.
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.20;
import { ERC721 } from "https://github.com/Vectorized/solady/blob/main/src/tokens/ERC721.sol";
import { IERC20 } from "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/interfaces/IERC20.sol";
import { ClonesWithImmutableArgs } from "https://github.com/wighawag/clones-with-immutable-args/blob/master/src/ClonesWithImmutableArgs.sol";
import { Clone } from "https://github.com/wighawag/clones-with-immutable-args/blob/master/src/Clone.sol";
/// @notice For the purpose of RACE-21 there are no bugs in the `Rabbits` token contract.
/// If anything exists it's due to integrating with the token contract, not the token contract itself.
contract Rabbits is ERC721 {
/* --------------------------------- Storage -------------------------------- */
string internal _name;
string internal _symbol;
address public deployer;
/* --------------------------------- Errors --------------------------------- */
error NotAuthorized();
/* -------------------------------- Modifiers ------------------------------- */
modifier onlyDeployer() {
if (msg.sender != deployer) {
revert NotAuthorized();
}
_;
}
/* ------------------------------- Constructor ------------------------------ */
constructor() {
_name = "RabidRabbits";
_symbol = "RABID";
deployer = msg.sender;
}
/* ---------------------------- Solady Overrides ---------------------------- */
/// @dev Returns the token collection name.
function name() public view override returns (string memory) {
return _name;
}
/// @dev Returns the token collection symbol.
function symbol() public view override returns (string memory) {
return _symbol;
}
/// @dev Returns the Uniform Resource Identifier (URI) for token `id`.
function tokenURI(uint256) public pure override returns (string memory) {
return "{ fancyJson: true }"; // not relevant for RACE-21
}
function mint(address to, uint256 tokenId) public onlyDeployer {
_mint(to, tokenId);
}
function sudoBurn(uint256 tokenId) public onlyDeployer {
_burn(tokenId);
}
function burn(uint256 tokenId) public {
_burn(msg.sender, tokenId);
}
}
interface SharedTypes {
/* ---------------------------------- Types --------------------------------- */
enum CommitReveal {
None,
Commit,
Reveal
}
struct Research {
CommitReveal commitReveal;
bool successfulFinding;
}
struct Entropy {
bytes32 entropy;
uint256 timestamp;
bytes32 previousEntropy;
bool previousResult;
}
}
/// @notice Uses clones-with-immutable-args to avoid cost of storage reads.
/// See https://github.com/wighawag/clones-with-immutable-args
contract ResearchLab is Clone {
/* --------------------------------- Storage -------------------------------- */
SharedTypes.Research[] public researchEndeavors;
SharedTypes.Entropy public entropy;
/* --------------------------------- Getters -------------------------------- */
/// @notice For clones-with-immutable-args custom getters are needed.
function getRandomOracle() public pure returns (address) {
return _getArgAddress(0);
}
function getEndeavor(uint256 idx) public view returns (SharedTypes.Research memory) {
return researchEndeavors[idx];
}
/* ------------------------ State Transition Function ----------------------- */
function commitReseachResources() public {
researchEndeavors.push(SharedTypes.Research(SharedTypes.CommitReveal.Commit, false));
}
function revealResearchResult(uint256 idx) public {
SharedTypes.Research storage endeavor = researchEndeavors[idx];
trulyRandomExternalCall(endeavor);
}
function trulyRandomExternalCall(SharedTypes.Research memory endeavor) internal {
// Update state at the same time
bytes memory payload = abi.encodeWithSelector(TrulyRandomOracleMock.oracleResult.selector, endeavor);
// Use TrulyRandomOracleMock to update state
getRandomOracle().delegatecall(payload);
}
}
contract TrulyRandomOracleMock {
/* --------------------------------- Storage -------------------------------- */
SharedTypes.Research[] internal researchEndeavors;
SharedTypes.Entropy public entropy;
/* ------------------------ State Transition Function ----------------------- */
// For the purpose of RACE-21 assume the random oracle obtains a seed from a truly random source.
function oracleResult(SharedTypes.Research memory endeavor) public {
// This line is a mock, assume it was actually random from external source.
bytes32 mockRandomSeed = keccak256(abi.encode(entropy, endeavor, block.timestamp));
// Calculate new entropy.
SharedTypes.Entropy memory newEntropy = entropy;
calculateResult(newEntropy, mockRandomSeed);
// Update entropy state.
entropy = newEntropy;
// Update endeavor state.
endeavor.commitReveal = SharedTypes.CommitReveal.Reveal;
endeavor.successfulFinding = newEntropy.previousResult;
}
/* --------------------------------- Helpers -------------------------------- */
function calculateResult(SharedTypes.Entropy memory newEntropy, bytes32 mockRandomSeed) internal view {
newEntropy.entropy = mockRandomSeed;
newEntropy.timestamp = block.timestamp;
newEntropy.previousEntropy = entropy.entropy;
// 1 in 10_000 chance but disallow 2 in a row
newEntropy.previousResult = uint256(mockRandomSeed) % 10_000 == 0 && !entropy.previousResult;
}
}
contract RabidRabbits {
using ClonesWithImmutableArgs for address;
/* --------------------------------- Errors --------------------------------- */
error NotDead();
/* ---------------------------------- Types --------------------------------- */
enum Rabies {
None,
Symptomatic,
Ded,
Cured
}
struct Bunny {
uint256 id;
Rabies rabies;
address owner;
uint256 birthed;
ResearchLab[] researchLabs;
}
/* -------------------------- Immutable / Constant -------------------------- */
uint256 public constant ADOPTION_PRICE = 10 ether;
uint256 public constant DR_FEES = 0.5 ether;
IERC20 public immutable lidoToken;
ResearchLab public immutable researchLabImpl;
address public immutable cloneArgsTarget;
/* --------------------------------- Storage -------------------------------- */
Bunny[] public bunnies;
Rabbits public rabbitToken;
/* ------------------------------- Constructor ------------------------------ */
// For the purpose of RACE-21 we are passing in the Lido token from mainnet
// https://etherscan.io/token/0x5a98fcbea516cf06857215779fd812ca3bef1b32#code
constructor(IERC20 _lidoToken, address _cloneArgsTarget) {
// Deploy contracts.
rabbitToken = new Rabbits();
researchLabImpl = new ResearchLab();
// Set contract state.
lidoToken = IERC20(_lidoToken);
cloneArgsTarget = _cloneArgsTarget;
}
/* --------------------------------- Getters -------------------------------- */
function labsOf(uint256 bunnyIdx, uint256 labIdx) public view returns (ResearchLab) {
return bunnies[bunnyIdx].researchLabs[labIdx];
}
/* ------------------------ Public State Transitions ------------------------ */
function adopt() public {
lidoToken.transferFrom(msg.sender, address(this), ADOPTION_PRICE);
rabbitToken.mint(msg.sender, bunnies.length);
// 1 in 1000 chance of having a rabbit with rabies.
uint256 randomSeed = uint256(blockhash(block.number));
Rabies rabies = randomSeed % uint32(1000) == 0 ? Rabies.Symptomatic : Rabies.None;
bunnies.push(Bunny(bunnies.length, rabies, msg.sender, block.timestamp, new ResearchLab[](0)));
}
function miracleCure(uint256 start, uint256 end) public {
// Once a cure is found we can cure symptomatic rabbits.
// "left as exercise for the reader"
}
function researchAndDevelopment(uint256 idx) public {
// Clone ResearchLab.
bytes memory cloneArgs = abi.encodePacked(cloneArgsTarget);
ResearchLab researchLab = ResearchLab(address(researchLabImpl).clone(cloneArgs));
// init multiple research initiatives.
for (uint256 i = 0; i < 10; i++) {
researchLab.commitReseachResources();
// This is a RACE, no time for actual commit/reveal.
researchLab.revealResearchResult(i);
}
// Credit where credit is due.
bunnies[idx].researchLabs.push(researchLab);
}
/// @notice if a rabbit has rabies and is not cured, it will die (╥﹏╥)
function burry(uint256 idx) public {
Bunny[] memory localBunnies = bunnies;
if (localBunnies[idx].rabies != Rabies.Symptomatic && (block.timestamp < 7 days + localBunnies[idx].birthed)) {
revert NotDead();
}
// Efficiently delete from array.
bunnies[idx] = bunnies[localBunnies.length - 1];
bunnies.pop();
// Tidy up and burn token.
rabbitToken.sudoBurn(idx);
}
}
Question 1 of 8
Which of the following statement(s) is/are true about the burry()
function?
- A. The function always reverts when an out of bounds index (
idx
) is passed - B. The function always reverts when
localBunnies[idx].rabies
is not Rabies.Symptomatic - C. The function always reverts when less than 7 days have passed since the Rabbit was minted
- D. None of the above
Solution
Correct is A.
- A: Solidity automatically checks for out-of-bounds index access and will revert.
- B/C: There is an error in the function, the
&&
should be||
as it should revert when either side is true. At the moment it would require both conditions to be true in order to revert.
Question 2 of 8
clones-with-immutable-args (opens in a new tab) are used to deploy ResearchLabs
. Which of the following statement(s) about this pattern is/are true?
- A. Arguments originating from the proxy to the implementation are immutable
- B. All arguments passed to the implementation are immutable
- C. Gas costs are lower when cloning versus deploying new implementations
- D. The implementation can be
selfdestruct
’ed by a malicious caller
Solution
Correct is A, C, D.
This is based on the Astaria disclosure (described here by @devtooligan (opens in a new tab)). Don’t let the name fool you, the proxy is where args are immutable; the implementation receives these arguments as calldata.
When called by the proxy, everything works as intended. When the implementation is called directly, the caller controls the arguments and delegatecalls
become particularly problematic.
- A: As long as the proxy is involved, the arguments originating directly from it are indeed immutable.
- B: The proxy may receive calldata from the
msg.sender
. It appends the immutable arguments to this before forwarding the call. - C: This is true because the bytecode of a proxy is usually much smaller and therefore cheaper to deploy than the actual implementation it points to (proxy clone pattern)
- D: The
ResearchLab
contract can be directly called (without a proxy passing a valid immutable argument). This means that the address it delegate-calls to can be pointed to a contract that makes it self-destruct.
Question 3 of 8
The adopt()
function has a random calculation. Which of the following statement(s), if any, about the source of randomness is/are true?
- A. By using Flashbots a caller can guarantee that
Bunny.rabies == Rabies.None
- B. There is an error present that can be corrected by hashing
randomSeed
before using it - C. Integer overflow introduces unexpected behavior in this function
- D. None of the above
Solution
Correct is D.
There is certainly a problem with the randomSeed
, it is not random at all. The use of blockhash
for the current block returns 0 (see evm.codes (opens in a new tab)) meaning randomSeed
behaves more like a constant than a random number.
- A: Misdirection: Not block hash will always be 0, can't be influenced using Flashbots.
- B: Misdirection: Hashing a constant value will just return another constant.
- C: Misdirection: No overflows possible in this Solidity version without the use of
unchecked
-blocks
Question 4 of 8
Which of the following statement(s) is/are true about the adopt()
function?
- A. Minting does not take place unless precisely
ADOPTION_PRICE
is paid inlidoToken
- B.
lidoToken
is a known token and is immutable in this contract, thereforetransferFrom
is safe to use - C. In some but not all cases the
burry()
function can cause theadopt()
function to be DOS’d - D. Duplicate bunnies array entries will be produced if they burn their Rabbits NFT using the Rabbits ERC721 contract directly instead of using the
RabidRabbits.burry()
function
Solution
Correct is C.
- A/B: The use of Lido token introduces MiniMe (opens in a new tab) Token to the system. These tokens do not revert on failure and instead return false. Even though a user cannot introduce a problematic token to the system, we have caused the issue ourselves in the constructor. The unchecked return value, i.e. not using
safeTransferFrom
, allows calls to succeed even when no tokens are transferred. - C:
burry()
reduces the length of the bunnies storage array. In the adopt functionbunnies.length
is used to decide which token id to mint. If any but the last bunnies index is buried, the array length decrements and the next call to adopt fails due to attempting to mint a duplicate token id. - D: While the array entries of those burned bunnies will remain, this won't cause duplicates as the array length isn't decreased, and therefore later bunnies will have a non-duplicate id.
Question 5 of 8
TrulyRandomOracleMock.oracleResult()
calls calculateResult()
passing a struct in memory. calculateResult()
attempts to modify the memory struct and does not return a value. Finally, revealResearchResult()
writes the entropy struct to storage; this is going to:
- A. Silently fail to update the
newEntropy
struct and will store the original - B. Correctly update and then store the
newEntropy
struct - C. Revert at runtime
- D. Fail at compile time
Solution
Correct is B.
When an internal function operates on memory, memory is updated in place. Contrasted with external calls where memory may be passed but is not updated in the calling contract.
- A/B: The
oracleResult
function callscalculateResult
and passes it the entropy state variable's value as a reference in memory. SincecalculateResult
is an internal function, it's able to adjust theEntropy
values asoracleResult
uses the same memory. This updated memory reference variable is then used to update the value in storage. - C/D: Misdirecting answers. It doesn't revert and compiles fine.
Question 6 of 8
Which of the following statement(s) about the constructor
is/are true?
- A. Gas can be saved by not double casting
lidoToken = IERC20(_lidoToken)
- B. Gas can be saved by making
rabbitToken
immutable - C. Gas can be saved by storing
cloneArgsTarget
as bytes - D.
_lidoToken
as an argument helps in deploying on multiple networks
Solution
Correct is B, D.
- A: At the bytecode level, casting is a noop and incurs no gas.
- B: Immutable is cheaper than reading from storage
- C: bytes would be a storage array (immutable variables cannot have a non-value type) meaning we would incur sload costs.
- D: True, we can reference bridged Lido on other networks when deploying there. Further, testing and fuzzing benefits exist when not hardcoding addresses.
Question 7 of 8
Which of the following statement(s) is/are true about the burry()
function?
- A. A user can DOS the
burry()
function for theiridx
if they burn their Rabbits NFT using the Rabbits ERC721 contract directly instead of using theRabidRabbits.burry()
function - B. The
Bunny.researchLabs
array is correctly reset for the deletedidx
- C. The entire
burry()
function can be DOS’d resulting in all calls to this function failing - D. None of the above
Solution
Correct is A, B, C.
A: This function attempts to burn, meaning that if a token is burned already, calls to burry will revert.
B: While possibly high in gas costs, the array is cleared and the index may be reused in the adopt
function.
C: Whatever we thought we were doing to be efficient in our array handling is negated by loading the entire array into memory with Bunny[] memory localBunnies = bunnies;
. With at least 5 storage reads per array entry, after a few thousand entries in the array, the block gas limit is hit trying to load it all into memory.
Question 8 of 8
Which of the following statement(s) is/are true about ResearchLab.revealResearchResult()
and the functions it interacts with?
- A. The Solidity compiler will fail to compile
- B. The storage for
entropy
is updated - C. The storage for
researchEndeavors[idx]
is updated - D. None of the above
Solution
Correct is B.
- A: Misdirection: This compiles without issues.
- B/C: This question is included as a reminder to be aware of storage and memory context when calling between functions. To step through the functions and argument passing:
revealResearchResult
references endeavor in storage then callstrulyRandomExternalCall
which loads the data into memory.TrulyRandomOracleMock.oracleResult
is delegatecall’ed again with the endeavor data passed as memory. Delegatecall meansTrulyRandomOracleMock.oracleResult
has access to all of our storage, includingentropy
which it updates in storage.endeavor
on the other hand is in memory, meaningresearchEndeavors[idx]
is not updated at all.