Neby
Smart Contract Security Assessment
September 13, 2024
SUMMARY
ABSTRACT
Dedaub was commissioned to perform a security audit of Neby**, a fork of Uniswap V3 that introduces new functionality aimed at enhancing AMM staking. NEBY implements lockups in the UniswapV3-staking mechanism and introduces a proprietary staking protocol for its native token, NEBY. In addition, NEBY features an auto-compounding tool, "Harvester," which autonomously claims staking rewards when advantageous and reinvests them to optimize position growth. The audit assesses the security and functionality of these components, ensuring their reliability and adherence to best practices within the DeFi ecosystem..
SETTING & CAVEATS
This audit report mainly covers the contracts of the at-the-time private repository https://github.com/NebyDex/neby-contracts of the Protocol at commit cf113592765c0cd862cdd02029b6d2733d15944a
.
2 auditors worked on the codebase for 6 days on the following contracts:
neby-contracts/
- harvester/
- contracts/
- Harvester.sol
- neby/
- contracts/
- Neby.sol
- ProtocolFeeManager.sol
- Staker.sol
- v3-staker/
- contracts/
- LiquidityPositionStaker.sol
The audit’s main target is security threats, i.e., what the community understanding would likely call "hacking", rather than the regular use of the protocol. Functional correctness (i.e. issues in "regular use") is a secondary consideration. Typically it can only be covered if we are provided with unambiguous (i.e. full-detail) specifications of what is the expected, correct behavior. In terms of functional correctness, we often trusted the code’s calculations and interactions, in the absence of any other specification. Functional correctness relative to low-level calculations (including units, scaling and quantities returned from external protocols) is generally most effectively done through thorough testing rather than human auditing.
VULNERABILITIES & FUNCTIONAL ISSUES
This section details issues affecting the functionality of the contract. Dedaub generally categorizes issues according to the following severities, but may also take other considerations into account such as impact or difficulty in exploitation:
- User or system funds can be lost when third-party systems misbehave.
- DoS, under specific conditions.
- Part of the functionality becomes unusable due to a programming error.
- Breaking important system invariants but without apparent consequences.
- Buggy functionality for trusted users where a workaround exists.
- Security issues which may manifest when the system evolves.
Issue resolution includes “dismissed” or “acknowledged” but no action taken, by the client, or “resolved”, per the auditors.
CRITICAL SEVERITY
[No critical severity issues]
HIGH SEVERITY
[No high severity issues]
MEDIUM SEVERITY
[No medium severity issues]
LOW SEVERITY
The Harvester::autoCompound
function charges a fee only when its caller wishes to do so, by passing the chargeFee
flag.
function autoCompound(
AutoCompoundParams memory params
)
...
// distribute fees - handle 2 cases (nft owner - no protocol reward / anyone else)
if (!params.chargeFee) {
reward0 = 0;
reward1 = 0;
} else {
uint256 protocolFees0 = state
.amount0Fees
.mul(protocolFractionOfRewardX64)
.div(Q64);
uint256 protocolFees1 = state
.amount1Fees
.mul(protocolFractionOfRewardX64)
.div(Q64);
reward0 = state.amount0Fees.sub(protocolFees0);
reward1 = state.amount1Fees.sub(protocolFees1);
_increaseHarvestBalance(msg.sender, state.token0, reward0);
_increaseHarvestBalance(msg.sender, state.token1, reward1);
_increaseHarvestBalance(owner(), state.token0, protocolFees0);
_increaseHarvestBalance(owner(), state.token1, protocolFees1);
}
...
}
This allows anyone who wishes to facilitate the process to call the function with chargeFee == true
, in return for a percentage of the fee. If the fees are substantial, the NFT’s owner can choose to call Harvester::autoCompound
themselves with chargeFee == false
, avoiding the fee.
However, the NFT owner cannot guarantee that his call will be executed first, anyone could frontrun the owner's transaction to get the fees for themselves.
To avoid frontrunning scenarios, it could be desirable to allow the owner to explicitly configure that they wish to call the function themselves without fees, and prevent other users from collecting them. Or, similarly, there could be a mechanism that allows only the owner to call the function for a certain number of blocks, and only after make it available to other users.
Finally, note that the same issues appears in Harvest::harvest
which also accepts a chargeFee
flag.
CENTRALIZATION ISSUES
It is often desirable for DeFi protocols to assume no trust in a central authority, including the protocol’s owner. Even if the owner is reputable, users are more likely to engage with a protocol that guarantees no catastrophic failure even in the case the owner gets hacked/compromised. We list issues of this kind below. (These issues should be considered in the context of usage/deployment, as they are not uncommon. Several high-profile, high-value protocols have significant centralization threats.)
Neby::mint():15
function mint(address to, uint256 amount) external onlyOwner {
_mint(to, amount);
}
Partially Resolved: The NEBY team has added a cap for the first year of the minting, and they have stated that “Before the year is up we plan to transfer ownership (and the mint role) to a smart contract which will handle future mint limits.”
N2
In ProtocolFeeManager all fees can be pulled by the owner by inserting a fake pool and taking advantage of the uniswapV3SwapCallback
By using the setApprovedPool
functionality the owner can arbitrarily pull any tokens out if they would want to.
ProtocolFeeManager::uniswapV3Callback():229
function uniswapV3SwapCallback(
int256 amount0Delta,
int256 amount1Delta,
bytes calldata _data
) external override {
if (amount0Delta <= 0 && amount1Delta <= 0) revert InvalidSwap();
address rewardToken = abi.decode(_data, (address));
// @dedaub: If the pool is a malicious contract the owner can pull
any token out
bool rewardTokenApprovedForPool = poolToRewardToken[msg.sender][
rewardToken
];
if (!rewardTokenApprovedForPool) revert UnauthorizedPool(msg.sender);
IERC20(rewardToken).safeTransfer(
msg.sender,
uint256(amount0Delta > 0 ? amount0Delta : amount1Delta)
);
}
Same note as N1
Resolved by the NEBY team by enforcing that the contracts must be pools that output WROSE or USDC.
OTHER / ADVISORY ISSUES
This section details issues that are not thought to directly affect the functionality of the project, but we recommend considering them.
The below comment states that at least one element should be in the lockups, however when referencing the code this is not true.
ILiquidityPositionStaker::IncentiveKey:33
/// @param lockups The lockups for the incentive. At least one is required. If no lockup is desired, just use a lockup with a duration of 0 and a multiplier of 1.
struct IncentiveKey {
IERC20Minimal rewardToken;
IUniswapV3Pool pool;
uint256 startTime;
uint256 endTime;
address refundee;
Lockup[] lockups;
}
The code is compiled with Solidity 0.7.6
. Version 0.7.6
, in particular, has some known bugs, which we do not believe affect the correctness of the contracts.
DISCLAIMER
The audited contracts have been analyzed using automated techniques and extensive human inspection in accordance with state-of-the-art practices as of the date of this report. The audit makes no statements or warranties on the security of the code. On its own, it cannot be considered a sufficient assessment of the correctness of the contract. While we have conducted an analysis to the best of our ability, it is our recommendation for high-value contracts to commission several independent audits, a public bug bounty program, as well as continuous security auditing and monitoring through Dedaub Security Suite.
ABOUT DEDAUB
Dedaub offers significant security expertise combined with cutting-edge program analysis technology to secure some of the most prominent protocols in DeFi. The founders, as well as many of Dedaub's auditors, have a strong academic research background together with a real-world hacker mentality to secure code. Protocol blockchain developers hire us for our foundational analysis tools and deep expertise in program analysis, reverse engineering, DeFi exploits, cryptography and financial mathematics.