Comment on page
👨💻
Audits
All audits of Buffer Contracts
Buffer protocol has been audited extensively by industry-leading smart contract auditor - Sherlock.xyz
All Audits of Buffer smart contracts can be found below:
In-Scope Contracts
- contracts/core/AccountRegistrar.sol
- contracts/core/Booster.sol
- contracts/core/BufferBinaryOptions.sol
- contracts/core/BufferBinaryPool.sol
- contracts/core/BufferRouter.sol
- contracts/core/CreationWindow.sol
- contracts/core/OptionMath.sol
- contracts/core/OptionConfig.sol
- contracts/core/Validator.sol
Deployment Chain(s)
- Arbitrum One Mainnet
Identifier | Title | Severity | Mitigated |
|---|---|---|---|
[H-01] | Payments for coupons to Booster are irretrievable | High | ✔️ |
[H-02] | BufferBinaryPool can permanently lock funds on early exercise | High | ✔️ |
[H-03] | Market direction signature can be abused if privateKeeperMode is disabled | High | ✔️ |
[H-04] | closeAnytime timestamp is never validated against current timestamp | High | ✔️ |
[H-05] | closeAnyTime timestamp is never validated against pricing timestamp | High | ✔️ |
[M-01] | booster#buy fails to apply discount | Medium | ✔️ |
[M-02] | Transferred options can only be closed early by previous owner | Medium | ✔️ |
[L-01] | Users can buy coupons for options that don't exist | Low | ✔️ |
[L-02] | registerAccount sub-call in BufferRouter#openTrades can revert entire transaction | Low | ✔️ |
[I-01] | Booster#buy should support buying multiple boosts at a time | Info | ✔️ |
[I-02] | OptionMath#_decay is never used | Info | ✔️ |
uint256 price = couponPrice - discount;
require(token.balanceOf(user) >= price, "Not enough balance");
token.safeTransferFrom(user, address(this), couponPrice); <- @audit tokens transferred to this
userBoostTrades[tokenAddress][user]
.totalBoostTrades += MAX_TRADES_PER_BOOST;
emit BuyCoupon(tokenAddress, user, couponPrice);
Booster#buy transfers tokens to itself but has no way to recover these tokens, causing them to be permanently trapped in this contract.
Transfer fees to a configurable address such as admin
if (option.expiration > closingTime) {
profit =
(option.lockedAmount *
OptionMath.blackScholesPriceBinary(
config.iv(),
option.strike,
closingPrice,
option.expiration - closingTime,
true,
isAbove
)) /
1e8;
} else {
profit = option.lockedAmount;
}
pool.send(optionID, user, profit);
When exercising an option early, it is possible to receive a partial payout depending on the factors such as when the contract is unlocked and the current asset price. This will cause the contract to send only a portion of the locked amount.
uint256 transferTokenXAmount = tokenXAmount > ll.amount
? ll.amount
: tokenXAmount;
ll.locked = false;
lockedPremium = lockedPremium - ll.premium;
lockedAmount = lockedAmount - transferTokenXAmount;
tokenX.safeTransfer(to, transferTokenXAmount);
This is problematic since the amount of tokens sent is also the amount unlocked. This will leave the remainder of the fees perpetually locked in the BufferBinaryPool contract.
Example: A user opens an option that locks 100 USDC. After some time they decide to exercise early. Due to current pricing factors their option is exercised for only 50 USDC. This unlocks 50 USDC leaving the other 50 USDC permanently locked, which means they can never be withdrawn by LPs
BufferBinaryPool#send should always unlock the entire option amount not just the amount sent.
Mitigated here. Upon exercise
option.lockedAmount is redeemed to the option contract. profit is sent to the user and the remainder (if any) is sent back to the pool. | if ( |
| !Validator.verifyMarketDirection( |
| params, |
| queuedTrade, |
| optionInfo.signer |
| ) |
| ) { |
| emit FailUnlock( |
| params.optionId, |
| params.targetContract, |
| "Router: Wrong market direction" |
| ); |
| continue; |
| } |
if (
!Validator.verifyMarketDirection(
params,
queuedTrade,
optionInfo.signer
)
) {
emit FailUnlock(
params.optionId,
params.targetContract,
"Router: Wrong market direction"
);
continue;
}
When options are exercised, the market direction is revealed via a signature provided by the opener. This can cause 2 significant issues if privateKeeperMode is disabled:
- 1.User can change the direction of their trade after to guarantee they win
- 2.User can open trade and withhold direction signature to indefinitely lock LP funds
Instead of using a signature at exercise, consider instead concatenating the direction with a salt then hashing storing that hash with the queued trade. Upon closure the salt and direction can be provided and the hash checked against the stored hash.
try
optionsContract.unlock(
params.optionId,
params.closingPrice,
publisherSignInfo.timestamp,
params.isAbove
)
{} catch Error(string memory reason) {
emit FailUnlock(params.optionId, params.targetContract, reason);
continue;
}
When exercising early, the timestamp used for unlocking is extremely important. The current implementation doesn't ever validate the current timestamp is anywhere near the current timestamp. If private keeper mode is disabled, a user could exercise their option in the past when they would get a better payoff.
Validate that exercise timestamp is within some margin of the current timestamp
Mitigated here by removing the ability to disable privateKeeperMode. This has only been addressed for non-trusted actors and can still be exploited by a malicious/compromised keeper
try
optionsContract.unlock(
params.optionId,
params.closingPrice,
publisherSignInfo.timestamp,
params.isAbove
)
{} catch Error(string memory reason) {
emit FailUnlock(params.optionId, params.targetContract, reason);
continue;
}
When an option is exercised early, it uses the timestamp of the pricing data without ever checking the timestamp of the closeAnytime signature. This can lead to 2 potential issues:
- 1.The user may receive less than expected due to the actual exercise timestamp being different than when they signed
- 2.If private keeper mode is disabled then transactions may be intercepted and the signature used by someone else to close the position with a much different timestamp
Pricing data timestamp and closeAnytime timestamp should be required to match
Mitigated here by removing the ability to disable privateKeeperMode. This has only been addressed for non-trusted actors and can still be exploited by a malicious/compromised keeper
uint256 price = couponPrice - discount;
re[Title](command:workbench.action.addRootFolder)quire(token.balanceOf(user) >= price, "Not enough balance");
token.safeTransferFrom(user, address(this), couponPrice); <- @audit transfers couponPrice rather than price
When buying boosts the contract mistakenly transfers couponPrice rather than price which is reduced by discount. Additionally the event also emits this incorrect value.
Transfer
price rather than couponPrice (address signer, ) = getAccountMapping(queuedTrade.user);
bool isUserSignValid = Validator.verifyCloseAnytime(
optionsContract.assetPair(),
closeParam.userSignInfo.timestamp,
params.optionId,
closeParam.userSignInfo.signature,
signer
);
Options are minted as NFTs allowing options to be transferred to other users. This allows users to potentially sell/transfer their options to other user. This cause 2 issues:
- 1.The previous owner can close the option early to cause damage to the new owner
- 2.The new owner cannot close their option early by themselves
NFT functionality should be removed or closeAnytime should determine the signer based on the owner of the option NFT
Booster#buy allows users to buy boosts for any tokens, which means that user can pay for and buy useless boost for tokens that aren't listed.
Consider limiting boost purchases to only tokens currently listed for better UX
Mitigated here the buy function was refactored to be callable only by owner and use permits for approval which negates the impact of this.
if (params[index].register.shouldRegister) {
accountRegistrar.registerAccount(
params[index].register.oneCT,
user,
params[index].register.signature
);
}
The openTrades function has been designed to prevent it from reverting under almost every circumstance. The exception to this is that accountRegistrar.registerAccount can revert the entire transaction.
Consider using a try-catch block to prevent it from reverting
Booster#buy only allows the purchase of one boost at a time. Consider allowing users to bulk buy boosts to save transactions and gas.
Consider allowing multiple boosts to be purchased in a single transaction
OptionMath#_decay is never used and should be removed
OptionMath#_decay should be removed
The core contributor team followed vigorous processes to develop and secure Buffer protocol, including
- Writing comprehensive unit tests
- Verifying contracts against written rules (a process known as Formal Verification)
- Conducting extensive internal code reviews
- Hiring reputable auditors to audit contracts
- Running a security bounty program
In the future, the core team will audit new versions of the protocol during major upgrades to the architecture of the protocol.
Last modified 2mo ago