Awesome
All Blockchain Audits
A collection of my personal blockchain security audits.
My publications on Substack.
My Youtube Account.
For enquiries and quotes on private audits or audit consultations, reach out to me on
BailSec
Date | Protocol Name |
---|---|
2024-12 | Undisclosed |
Shieldify Security
Date | Protocol Name |
---|---|
2024-12 | Undisclosed |
2024-12 | Undisclosed |
2024-11 | Dinero |
2024-11 | HoneyPot Finance |
2024-10 | Vyper |
Pashov Audit Group
Date | Protocol Name |
---|---|
2024-11 | Undisclosed |
2024-11 | Rivus |
2024-10 | LaPoste |
2024-10 | Ethena |
2024-09 | Interpol |
2024-07 | Curio |
2024-07 | GroupCoin |
2024-06 | Bio |
2024-05 | Fyde |
2024-05 | Aburra |
2024-05 | Rivus |
2024-04 | Open Dollar |
2024-04 | Curio |
2024-03 | Fyde |
2024-02 | Tapioca DAO |
Cantina
Date | Protocol Name | High | Medium | Low | Rank |
---|---|---|---|---|---|
2024-07 | Grass | 2 | - | - | 8 |
2024-04 | Curvance | 4 | - | - | 9 |
Code4rena
Date | Protocol Name | High | Medium | Low | Non-Critical | Gas | Rank |
---|---|---|---|---|---|---|---|
2024-05 | Renzo | 2 | 3 | - | - | - | 16/122 |
2024-02 | UniStaker | - | - | - | - | - | 12/47 |
2024-01 | Decent | 2 | 3 | - | - | - | 11/113 |
2024-01 | Salty.IO | 1 | 3 | - | - | - | 12/177 |
2023-12 | Shell | - | - | - | - | - | 1/22 |
2023-10 | Ethena | - | 3 | - | - | - | 1/147 |
2023-06 | Stader | - | 2 | - | - | - | 20/75 |
2023-05 | Venus | - | 3 | - | - | - | 12/102 |
2023-04 | Frankencoin | - | 2 | - | - | - | 52/199 |
2023-04 | Caviar | - | 1 | - | - | - | 31/120 |
2023-03 | Asymmetry | 2 | 3 | 8 | 4 | - | 17/246 |
2023-03 | Wenwin | - | 1 | 6 | 8 | - | 21/93 |
2023-02 | Popcorn | 3 | - | 4 | 2 | - | 54/169 |
2023-01 | RabbitHole Quest | 1 | 3 | 3 | 6 | - | 26/173 |
2023-01 | Ondo Finance | 1 | - | 4 | 2 | - | 6/69 |
2023-01 | Biconomy | - | 3 | 5 | 3 | - | 21/105 |
2022-11 | Debt DAO | - | 1 | 1 | 5 | 4 | 41/120 |
2022-10 | Holograph | - | 1 | 2 | 3 | 2 | 35/144 |
2022-10 | 3xcalibur | - | 2 | 1 | 3 | 1 | 26/102 |
2022-10 | Inverse Finance | - | 2 | - | - | - | 84/127 |
2022-10 | Juicebox | - | - | - | 3 | - | 35/67 |
2022-09 | VTVL Protocol | - | 1 | 1 | - | 4 | 79/198 |
2022-09 | Frax Finance | - | - | - | - | 8 | 99/133 |
2022-09 | Y2k Finance | - | - | - | - | 6 | 102/110 |
2022-09 | PartyDAO | - | - | - | - | 2 | 97/110 |
Sherlock
Date | Protocol Name | High | Medium |
---|---|---|---|
2023-01 | UXD Protocol | 1 | 3 |
2022-11 | Buffer Finance | - | 1 |
2022-11 | Rage Trade | 1 | - |
2022-11 | Astaria | 1 | 2 |
2022-10 | Union Finance | - | 1 |
Sherlock Judging Contest
Date | Protocol Name | Rank |
---|---|---|
2023-02 | Union Finance Update | 4/50 |
2023-01 | Ajna | 5/87 |
2023-01 | Cooler | 5/68 |
2023-01 | UXD Protocol | 10/69 |
2023-01 | Illuminate Round 2 | 12/33 |
2023-01 | Notional Update | 12/57 |
About Me
Top 76th Code4rena 2023 Leaderboards
Warden Name: peanuts
Sample Report (Biconomy)
Contest: https://code4rena.com/contests/2023-01-biconomy-smart-contract-wallet-contest
Findings Summary
ID | Title | Severity |
---|---|---|
M-01 | handleAggregatedOps() does not handle non-atomic transactions which results in whole function revert if one transaction does not go through | Medium |
M-02 | EntryPoint address in SmartAccount.sol cannot execute certain functions because of the onlyOwner modifier | Medium |
M-03 | Protocol has upgradeable contracts that cannot be upgraded | Medium |
L-01 | Initializers can be frontrunnable | Low |
L-02 | Contract should adhere to two-step ownership process | Low |
L-03 | Unused/Empty receive()/fallback() function | Low |
L-04 | OpenZeppelin's ECDSA is imported but not utilized | Low |
L-05 | Import exact functions instead of the whole contract itself | Low |
N-01 | Non-library/interface files should use fixed compiler versions, not floating ones | Info |
N-02 | Avoid the use of sensitive terms in favour of neutral ones | Info |
N-03 | Testing coverage is not adequate enough | Info |
Full Report
Medium Findings:
[M-01] handleAggregatedOps() does not handle non-atomic transactions which results in whole function revert if one transaction does not go through
Lines of code
https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol#L93 https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol#L385 https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/aa-4337/core/EntryPoint.sol#L289
Impact
Function reverts if one account or paymaster is not validated, which leads to a waste of time and gas.
Proof of Concept
EntryPoint.UserOpsPerAggregator() takes in an array of opsPerAggregator in its parameter and loops through each struct. In the function, the function _validatePrepayment() is called which validates the account and paymaster from each opsPerAggregator.
function _validatePrepayment(uint256 opIndex, UserOperation calldata userOp, UserOpInfo memory outOpInfo, address aggregator)
private returns (address actualAggregator, uint256 deadline) {
uint256 preGas = gasleft();
MemoryUserOp memory mUserOp = outOpInfo.mUserOp;
_copyUserOpToMemory(userOp, mUserOp);
outOpInfo.userOpHash = getUserOpHash(userOp);
// validate all numeric values in userOp are well below 128 bit, so they can safely be added
// and multiplied without causing overflow
uint256 maxGasValues = mUserOp.preVerificationGas | mUserOp.verificationGasLimit | mUserOp.callGasLimit |
userOp.maxFeePerGas | userOp.maxPriorityFeePerGas;
require(maxGasValues <= type(uint120).max, "AA94 gas values overflow");
uint256 gasUsedByValidateAccountPrepayment;
(uint256 requiredPreFund) = _getRequiredPrefund(mUserOp);
(gasUsedByValidateAccountPrepayment, actualAggregator, deadline) = _validateAccountPrepayment(opIndex, userOp, outOpInfo, aggregator, requiredPreFund);
//a "marker" where account opcode validation is done and paymaster opcode validation is about to start
// (used only by off-chain simulateValidation)
numberMarker();
The function then continues to call _validateAccountPrepayment which does another round of checks.
function _validateAccountPrepayment(uint256 opIndex, UserOperation calldata op, UserOpInfo memory opInfo, address aggregator, uint256 requiredPrefund)
internal returns (uint256 gasUsedByValidateAccountPrepayment, address actualAggregator, uint256 deadline) {
unchecked {
uint256 preGas = gasleft();
MemoryUserOp memory mUserOp = opInfo.mUserOp;
address sender = mUserOp.sender;
_createSenderIfNeeded(opIndex, opInfo, op.initCode);
if (aggregator == SIMULATE_FIND_AGGREGATOR) {
numberMarker();
if (sender.code.length == 0) {
// it would revert anyway. but give a meaningful message
revert FailedOp(0, address(0), "AA20 account not deployed");
}
if (mUserOp.paymaster != address(0) && mUserOp.paymaster.code.length == 0) {
// it would revert anyway. but give a meaningful message
revert FailedOp(0, address(0), "AA30 paymaster not deployed");
}
try IAggregatedAccount(sender).getAggregator() returns (address userOpAggregator) {
aggregator = actualAggregator = userOpAggregator;
} catch {
aggregator = actualAggregator = address(0);
}
}
If any opsPerAggregator is not validated, the whole function will revert, leading to a waste of time and gas.
Tools Used
VSCode
Recommended Mitigation Steps
Make the loop non-atomic by using a try/catch block. If the paymaster / account is not validated, skip the loop and validate the next one. In _compensate, make sure to count the collected amount correctly if some opsPerAggregator are skipped because of validation failure.
[M-02] EntryPoint address in SmartAccount.sol cannot execute certain functions because of the onlyOwner modifier
Lines of code
https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L460 https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L465 https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccount.sol#L494
Impact
EntryPoint address in SmartAccount.sol cannot execute certain functions because of the onlyOwner modifier.
Proof of Concept The functions SmartAccount.execute() and SmartAccount.executeBatch() has an onlyOwner modifier.
function execute(address dest, uint value, bytes calldata func) external onlyOwner{
function executeBatch(address[] calldata dest, bytes[] calldata func) external onlyOwner{
However, both functions call another function _requireFromEntryPointOrOwner();
_requireFromEntryPointOrOwner();
which checks if msg.sender is adderss(entryPoint()) or owner
function _requireFromEntryPointOrOwner() internal view {
require(msg.sender == address(entryPoint()) || msg.sender == owner, "account: not Owner or EntryPoint");
}
Since execute() and executeBatch() has an onlyOwner modifier which checks if msg.sender is owner only,
modifier onlyOwner {
require(msg.sender == owner, "Smart Account:: Sender is not authorized");
_;
}
address(entryPoint()) cannot call the function execute() and executeBatch().
Tools Used
VSCode
Recommended Mitigation Steps
Either take out the onlyOwner modifer in the two functions or remove the _requireFromEntryPointOrOwner() check if the function intends to not allow the msg.sender to be address(entryPoint()).
[M-03] Protocol has upgradeable contracts that cannot be upgraded
Lines of code
Impact
Protocol cannot upgrade the upgradeable contract.
Proof of Concept
SmartAccount.sol uses ReentrancyGuardUpgradeable but not UUPSUpgradeable.
contract SmartAccount is
Singleton,
BaseSmartAccount,
IERC165,
ModuleManager,
SignatureDecoder,
SecuredTokenTransfer,
ISignatureValidatorConstants,
FallbackManager,
Initializable,
ReentrancyGuardUpgradeable
{
Tools Used
Manual Review
Recommended Mitigation Steps
import UUPSUpgradeable, inherit the contract, and initialize the contract
import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
contract SmartAccount is
Singleton,
BaseSmartAccount,
IERC165,
ModuleManager,
SignatureDecoder,
SecuredTokenTransfer,
ISignatureValidatorConstants,
FallbackManager,
Initializable,
ReentrancyGuardUpgradeable
+ UUPSUpgradeable
{
function init(address _owner, address _entryPointAddress, address _handler) public override initializer {
require(owner == address(0), "Already initialized");
require(address(_entryPoint) == address(0), "Already initialized");
require(_owner != address(0),"Invalid owner");
require(_entryPointAddress != address(0), "Invalid Entrypoint");
require(_handler != address(0), "Invalid Entrypoint");
+ __UUPSUpgradeable_init();
owner = _owner;
_entryPoint = IEntryPoint(payable(_entryPointAddress));
if (_handler != address(0)) internalSetFallbackHandler(_handler);
setupModules(address(0), bytes(""));
}
Also add storage gap for upgradeable contracts.
uint256[45] private __gap;
Low and Non-Critical Findings:
[L-01] Initializers can be frontrunnable
If the initializer is not executed in the same transaction as the constructor, a malicious user can front-run the initialize() call, forcing the contract to be redeployed.
[L-02] Contract should adhere to two-step ownership process
Setting the owner within one function can be dangerous if the address of the owner in the parameter is not set properly. Consider using a two-step process whereby the owner has to acknowledge the change before changing the ownership.
[L-03] Unused/Empty receive()/fallback() function
If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth)). Having no access control on the function means that someone may send Ether to the contract, and have no way to get anything back out, which is a loss of funds
[L-04] OpenZeppelin's ECDSA is imported but not utilized
Use OpenZeppelin’s ECDSA contract rather than calling ecrecover() directly
[L-05] Import exact functions instead of the whole contract itself
Example:
import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; instead of
import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; Applies to all contracts in Biconomy protocol.
SmartAccount.sol:
import "./libs/LibAddress.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol"; import "./BaseSmartAccount.sol"; import "./common/Singleton.sol"; import "./base/ModuleManager.sol"; import "./base/FallbackManager.sol"; import "./common/SignatureDecoder.sol"; import "./common/SecuredTokenTransfer.sol"; import "./interfaces/ISignatureValidator.sol"; import "./interfaces/IERC165.sol"; import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
[N-01] Non-library/interface files should use fixed compiler versions, not floating ones
eg. 0.8.10 instead of ^0.8.10
[N-02] Avoid the use of sensitive terms in favour of neutral ones
Use allowlist rather than whitelist.
[N-03] Testing coverage is not adequate enough
What is the overall line coverage percentage provided by your tests?: 41 Protocol should aim to achieve >95% testing coverage to make sure that all functions are in working order.