- None beyond the code repositories
- Review Summary
- Code Evaluation Matrix
- Findings Explanation
- Critical Findings
- High Findings
- Medium Findings
- Low Findings
- Gas Savings Findings
- Informational Findings
- Final remarks
Bunni Zap provides zapping contracts for making multiple Bunni interactions in a single transaction.
The contracts of the Bunni Zap Repo were reviewed over 10 days. The code review was performed by 2 auditors between March 5, 2023 and March 15, 2023. The repository was under active development during the review, but the review was limited to the latest commit at the start of the review. This was commit 4ab7c00f178ea16f5eaba0eca0884a644d99c02c for the Bunni Zap repo.
The scope of the review consisted of the following contracts at the specific commit:
After the findings were presented to the Bunni Zap team, fixes were made and included in several PRs.
This review is a code review to identify potential vulnerabilities in the code. The reviewers did not investigate security practices or operational security and assumed that privileged accounts could be trusted. The reviewers did not evaluate the security of the code relative to a standard or specification. The review may not have identified all potential attack vectors or areas of vulnerability.
yAudit and the auditors make no warranties regarding the security of the code and do not warrant that the code is free from defects. yAudit and the auditors do not represent nor imply to third parties that the code has been audited nor that the code is free from defects. By deploying or using the code, Bunni Zap and users of the contracts agree to use the code at their own risk.
|Access Control||Good||All functions of the
|Complexity||Minor||All of the functions in the
|Decentralization||Good||There are no privileged actors in the
|Code stability||Good||No changes were made to the code over the course of the audit.|
|Documentation||Medium||The only documentation available for the project is the README, however the functionality in
|Monitoring||Average||The underlying vault, gauge, and bunni hub dependencies emit events, however none of the
|Testing and verification||Low||The
Findings are broken down into sections by their respective impact:
- Critical, High, Medium, Low impact
- These are findings that range from attacks that may cause loss of funds, impact control/ownership of the contracts, or cause any unintended consequences/actions that are outside the scope of the requirements
- Gas savings
- Findings that can improve the gas efficiency of the contracts
- Findings including recommendations and best practices
enterWithVaultShares() revoke token approval after the function has finished executing.
Low. It is unlikely that this will result in loss of funds, but is still considered best practice.
Add token revocation everywhere token approval is granted.
Fixed in commit efba894b7e4c136008b065b2a7675e0a1c3f96cb
Several functions are using the
nonReentrant modifier but aren’t manipulable with re-entrancy.
nonReentrant is usually needed when a function is manipulating state variables that are used in other functions. In this case, the functions listed below are not manipulating state variables that are used in other functions, so the
nonReentrant modifier is not needed.
List of functions:
nonReentrant modifier from the functions listed above.
Fixed in commit 96c5c7342e2c91afedbf8734bdfb32dbbe1cd5e9
Several external functions are marked as
payable but are not used as such.
The following functions are marked as
payable but are not used as such:
Informational. This is not a vulnerability but it is a good practice to remove the payable modifier if it is not used.
payable modifier from the functions listed above.
Acknowledged, no change. When
wrapEthInput() is part of a multicall,
msg.value of the multicall is non-zero, so other functions used in the multicall must be payable to avoid reverting.
While the underlying dependencies in each function of the
BunniLpZapIn contract emit events, the functions themselves do not emit events.
Add events to the end of each
Acknowledged, no change. We don’t really expect to index data from this contract, and even if we do want to do so in the future we can just redeploy with events added.
Our primary concern is related to the assumption that the
BunniLpZapIn contract will not hold any balances. An invariant test was written during the audit to ensure that this invariant is maintained and the intended use of this contract is that
zapIn() is the last call in the multicall, which returns the token balances to the recipient. However, if this assumption is ever broken due to faulty configuration or input, the balances in the contract can easily be withdrawn by an attacker using the
useContractBalance boolean, or nefarious input.
Aside from this, the contract is relatively small and fairly straightforward due to the assumption that balances will not be held in the contract. For this reason, the contract is considered relatively safe, as is evident by the lack of high, critical, and medium findings.