-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
caf2a6f
commit 2092ed6
Showing
574 changed files
with
52,116 additions
and
10 deletions.
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
Narrow Macaroon Goblin | ||
|
||
High | ||
|
||
# Attacker can make auction failure by bidding just before auction end. | ||
|
||
### Summary | ||
|
||
Just before the auction ends, if the bidCount equals maxBids and the currentCouponAmount matches the totalBuyCouponAmount, the attacker places a small bid with a higher ratio than the lowest bid. In this scenario, the lowest bid is removed, causing the currentCouponAmount to fall below the totalBuyCouponAmount. As a result, the auction transitions to the State.FAILED_UNDERSOLD state. | ||
|
||
### Root Cause | ||
|
||
https://github.com/sherlock-audit/2024-12-plaza-finance-0xlu7/blob/227f7e7dbd2435cfa1ac940341453c728e323b03/plaza-evm/src/Auction.sol#L157 | ||
Before the auction ends, if the new bid's ratio is bigger than lowest, the lowest is removed. | ||
https://github.com/sherlock-audit/2024-12-plaza-finance-0xlu7/blob/227f7e7dbd2435cfa1ac940341453c728e323b03/plaza-evm/src/Auction.sol#L340 | ||
In this case, the amount of the new bid is less than the lowest bid, causing the currentCouponAmount to become less than the totalBuyCouponAmount. | ||
|
||
|
||
### Internal Pre-conditions | ||
|
||
_No response_ | ||
|
||
### External Pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
Just before the auction ends, if the bidCount equals maxBids and the currentCouponAmount matches the totalBuyCouponAmount, the attacker places a small bid with a higher ratio than the lowest bid. In this scenario, the lowest bid is removed, causing the currentCouponAmount to fall below the totalBuyCouponAmount. As a result, the auction transitions to the State.FAILED_UNDERSOLD state. | ||
|
||
### Impact | ||
|
||
The auction fails. | ||
|
||
### PoC | ||
|
||
```solidity | ||
function testAttackScenario() public { | ||
//maxBids is 2. | ||
//totalBuyCouponAmount is 5000. | ||
vm.startPrank(alice1); // first | ||
usdc.mint(alice1, 3000 ether); | ||
usdc.approve(address(auction), 3000 ether); | ||
auction.bid(100 ether, 2000 ether); // Legitimate bid | ||
vm.stopPrank(); | ||
vm.startPrank(alice2); // second | ||
usdc.mint(alice2, 2000 ether); | ||
usdc.approve(address(auction), 2000 ether); | ||
auction.bid(150 ether, 2000 ether); | ||
vm.stopPrank(); | ||
vm.warp(block.timestamp + 12 days - 1 hours); //just before ends | ||
vm.startPrank(attacker); // attacker | ||
usdc.mint(attacker, 30 ether); | ||
usdc.approve(address(auction), 30 ether); | ||
auction.bid(1 ether ,30 ether); | ||
vm.stopPrank(); | ||
vm.warp(block.timestamp + 1 days); // auction ends | ||
vm.prank(pool); | ||
auction.endAuction(); | ||
assertEq(uint256(auction.state()), uint256(Auction.State.FAILED_UNDERSOLD)); | ||
``` | ||
|
||
### Mitigation | ||
|
||
There has to be a validation based on time. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
Basic Lilac Marmot | ||
|
||
High | ||
|
||
# Token duplication verification failure will enable double withdrawals | ||
|
||
### Summary | ||
|
||
# Summary | ||
|
||
```solidity | ||
function debondAndLock(address _pod, uint256 _amount) external nonReentrant { | ||
require(_amount > 0, "D1"); | ||
require(_pod != address(0), "D2"); | ||
IERC20(_pod).safeTransferFrom(_msgSender(), address(this), _amount); | ||
IDecentralizedIndex _podContract = IDecentralizedIndex(_pod); | ||
IDecentralizedIndex.IndexAssetInfo[] memory _podTokens = _podContract.getAllAssets(); | ||
address[] memory _tokens = new address[](_podTokens.length); | ||
uint256[] memory _balancesBefore = new uint256[](_tokens.length); | ||
**// Get token addresses and balances before debonding | ||
for (uint256 i = 0; i < _tokens.length; i++) { | ||
_tokens[i] = _podTokens[i].token; | ||
_balancesBefore[i] = IERC20(_tokens[i]).balanceOf(address(this)); | ||
} | ||
_podContract.debond(_amount, new address[](0), new uint8[](0)); | ||
uint256[] memory _receivedAmounts = new uint256[](_tokens.length); | ||
for (uint256 i = 0; i < _tokens.length; i++) { | ||
_receivedAmounts[i] = IERC20(_tokens[i]).balanceOf(address(this)) - _balancesBefore[i]; | ||
}** | ||
... | ||
} | ||
``` | ||
|
||
The `debondAndLock` function is designed to deposit a specific token. Since the `_pod` parameter can have an arbitrary address, the return value of the `getAllAssets` function can also vary depending on the address. If the `getAllAssets` function returns duplicate addresses, a single deposit could result in multiple values for `_receivedAmounts`. As a result, the deposited amount could be withdrawn multiple times, creating a vulnerability that allows multiple withdrawals from a single deposit. | ||
|
||
```solidity | ||
function _withdraw(address _user, uint256 _lockId) internal { | ||
LockInfo storage _lock = locks[_lockId]; | ||
require(_lock.user == _user, "W1"); | ||
require(!_lock.withdrawn, "W2"); | ||
require(block.timestamp >= _lock.unlockTime, "W3"); | ||
_lock.withdrawn = true; | ||
for (uint256 i = 0; i < _lock.tokens.length; i++) { | ||
if (_lock.amounts[i] > 0) { | ||
**IERC20(_lock.tokens[i]).safeTransfer(_user, _lock.amounts[i]);** | ||
} | ||
} | ||
emit TokensWithdrawn(_lockId, _user, _lock.tokens, _lock.amounts); | ||
} | ||
``` | ||
|
||
Since there is no validation for duplicate addresses, as described above, it becomes possible to make multiple withdrawals. | ||
|
||
### Root Cause | ||
|
||
in `PodUnwrapLocker.sol:74` there is a missing check on token duplication | ||
|
||
### Internal Pre-conditions | ||
|
||
_No response_ | ||
|
||
### External Pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
1. Deploy a contract that implements the transferFrom, getAllAssets, and debond functions. | ||
2. Call the debondAndLock function and set the _pod parameter to the address of the contract deployed in step 1. | ||
3. Withdraw the duplicate tokens through the earlyWithdraw function. | ||
|
||
### Impact | ||
|
||
The affected party will lose all assets. | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
```solidity | ||
// Get token addresses and balances before debonding | ||
for (uint256 i = 0; i < _tokens.length; i++) { | ||
_tokens[i] = _podTokens[i].token; | ||
**for(uint256 j=i+1; j<_tokens.length; j++) require(_tokens[i] != _tokens[j], "invalid same token");** | ||
_balancesBefore[i] = IERC20(_tokens[i]).balanceOf(address(this)); | ||
} | ||
``` | ||
|
||
performs checks to verify the token quantity and ensure that the same token is being used. | ||
|
||
# Refernces | ||
|
||
https://github.com/sherlock-audit/2025-01-peapods-finance/blob/main/contracts/contracts/PodUnwrapLocker.sol#L72-L75 | ||
|
||
https://github.com/sherlock-audit/2025-01-peapods-finance/blob/main/contracts/contracts/PodUnwrapLocker.sol#L158 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
Basic Lilac Marmot | ||
|
||
High | ||
|
||
# Due to the lack of amountOutMin setting, the attacker can steal tokens. | ||
|
||
### Summary | ||
|
||
# Summary | ||
|
||
```solidity | ||
function claimReward(address _wallet) external override { | ||
_processFeesIfApplicable(); | ||
_distributeReward(_wallet); | ||
emit ClaimReward(_wallet); | ||
} | ||
function _processFeesIfApplicable() internal { | ||
IDecentralizedIndex(INDEX_FUND).processPreSwapFeesAndSwap(); | ||
} | ||
``` | ||
|
||
Anyone can call the `processPreSwapFeesAndSwap` function of `INDEX_FUND` through the `claimReward` function. | ||
|
||
```solidity | ||
/// @notice The ```processPreSwapFeesAndSwap``` function allows the rewards CA for the pod to process fees as needed | ||
function processPreSwapFeesAndSwap() external override lock { | ||
require(_msgSender() == IStakingPoolToken(lpStakingPool).POOL_REWARDS(), "R"); | ||
_processPreSwapFeesAndSwap(); | ||
} | ||
``` | ||
|
||
```solidity | ||
/// @notice The ```_feeSwap``` function processes built up fees by converting to pairedLpToken | ||
/// @param _amount Number of pTKN being processed for yield | ||
function _feeSwap(uint256 _amount) internal { | ||
_approve(address(this), address(DEX_HANDLER), _amount); | ||
address _rewards = IStakingPoolToken(lpStakingPool).POOL_REWARDS(); | ||
uint256 _pairedLpBalBefore = IERC20(PAIRED_LP_TOKEN).balanceOf(_rewards); | ||
**DEX_HANDLER.swapV2Single(address(this), PAIRED_LP_TOKEN, _amount, 0, _rewards);** | ||
if (PAIRED_LP_TOKEN == lpRewardsToken) { | ||
uint256 _newPairedLpTkns = IERC20(PAIRED_LP_TOKEN).balanceOf(_rewards) - _pairedLpBalBefore; | ||
if (_newPairedLpTkns > 0) { | ||
ITokenRewards(_rewards).depositRewardsNoTransfer(PAIRED_LP_TOKEN, _newPairedLpTkns); | ||
} | ||
} else if (IERC20(PAIRED_LP_TOKEN).balanceOf(_rewards) > 0) { | ||
ITokenRewards(_rewards).depositFromPairedLpToken(0); | ||
} | ||
} | ||
``` | ||
|
||
When the `processPreSwapFeesAndSwap` function is executed, it triggers the `swapV2Single` function within the `_feeSwap` function of `DEX_HANDLER`. This function likely calls the Uniswap V2 swap function to perform a token swap. | ||
|
||
```solidity | ||
function swapV2Single( | ||
address _tokenIn, | ||
address _tokenOut, | ||
uint256 _amountIn, | ||
uint256 _amountOutMin, | ||
address _recipient | ||
) external | ||
``` | ||
|
||
The fourth parameter of the swapV2Single function is used to validate the minimum amount of tokens expected after the swap. However, in the _feeSwap function, this parameter is being passed as 0, which effectively means there is no minimum amount check. | ||
|
||
Through this, a malicious user could exploit the lack of a minimum amount check (by passing 0 as the fourth parameter) to steal fees. | ||
|
||
### Root Cause | ||
|
||
in `DecentralizedIndex.sol:232` amountOutMin is set to zero. | ||
|
||
|
||
### Internal Pre-conditions | ||
|
||
_No response_ | ||
|
||
### External Pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
1. The attacker calculates the swap amount of the DecentralizedIndex contract and adjusts the liquidity of the POOL. | ||
2. By calling the claimReward function, the attacker causes the DecentralizedIndex contract to swap, resulting in a loss of _rewards worth of tokens and returning tokens close to zero. | ||
|
||
### Impact | ||
|
||
The affected party continuously loses tokens equivalent to _rewards. | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
Even if the claimReward function is restricted to authorized users, a malicious actor could still exploit the system via front-running to steal fees. | ||
|
||
To prevent this, it is essential to validate the minimum amount during the swap. This would ensure that the swap cannot proceed unless the expected minimum amount of tokens is received, preventing a malicious user from benefiting from an unfavorable swap rate. | ||
|
||
# References | ||
|
||
https://github.com/sherlock-audit/2025-01-peapods-finance/blob/main/contracts/contracts/TokenRewards.sol#L137-L139 | ||
|
||
https://github.com/sherlock-audit/2025-01-peapods-finance/blob/main/contracts/contracts/DecentralizedIndex.sol#L232 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
Rhythmic Azure Manatee | ||
|
||
Medium | ||
|
||
# The bulkProcessPendingYield function is external and callable by any account | ||
|
||
### The bulkProcessPendingYield function is external and callable by any account | ||
https://github.com/sherlock-audit/2025-01-peapods-finance/blob/main/contracts/contracts/BulkPodYieldProcess.sol#L20-L26 | ||
|
||
### Description | ||
The bulkProcessPendingYield function is external and callable by any account. This design choice can be deliberate (e.g., a public keeper pattern), but it also opens the possibility of repeated or spam calls. An actor—malicious or not—could invoke this function at will, potentially causing operational overhead or consuming block gas limits. | ||
|
||
### Impact (DoS / Spamming Concern) | ||
- **Spam Calls:** Repeated invocations by a malicious actor might bloat transaction history and cause elevated gas costs for legitimate users or keepers. | ||
- **DoS Vector:** If the underlying ITokenRewards logic is gas-intensive or triggers state updates that can fail, repeated spamming could temporarily block or disrupt legitimate yield-harvesting calls. | ||
|
||
### Proof of Concept | ||
1. **Setup:** Deploy BulkPodYieldProcess and a mock IDecentralizedIndex and ITokenRewards contract. | ||
2. **Repeated Calls:** | ||
```solidity | ||
// Attacker script, pseudo-code: | ||
for (uint i = 0; i < 100; i++) { | ||
bulkPodYieldProcess.bulkProcessPendingYield(indexArray); | ||
} | ||
``` | ||
3. **Result:** | ||
- The contract accepts each call (no restriction). | ||
- If the yield-processing function is gas-intensive, repeated calls might cause short-term DoS for other functions on the network or consume substantial gas from any keepers trying to do a genuine harvest. | ||
|
||
**Foundry Test** | ||
Append this foundry test below to the following file and run forge test: contracts/test/BulkPodYieldProcess.t.sol. | ||
```solidity | ||
// function is called by anyone | ||
function testBulkProcessPendingYieldByAnyone() public { | ||
IDecentralizedIndex[] memory idxArray = new IDecentralizedIndex[](indices.length); | ||
for (uint256 i = 0; i < indices.length; i++) { | ||
idxArray[i] = indices[i]; | ||
} | ||
vm.startPrank(address(0xfeefdeef)); | ||
processor.bulkProcessPendingYield(idxArray); | ||
vm.stopPrank(); | ||
// Verify each index's staking pool and rewards were accessed correctly | ||
for (uint256 i = 0; i < indices.length; i++) { | ||
assertEq(indices[i].lpStakingPool(), stakingPools[i]); | ||
assertEq(MockStakingPoolToken(stakingPools[i]).POOL_REWARDS(), poolRewards[i]); | ||
} | ||
} | ||
``` | ||
|
||
**Log Results Of Foudry Test** | ||
```log | ||
# Test function is called by anyone | ||
forge test --match-test testBulkProcessPendingYieldByAnyone | ||
Ran 1 test for test/BulkPodYieldProcess.t.sol:BulkPodYieldProcessTest | ||
[PASS] testBulkProcessPendingYieldByAnyone() (gas: 98677) | ||
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.12ms (437.58µs CPU time) | ||
Ran 1 test suite in 144.57ms (2.12ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests) | ||
``` | ||
### Recommendation | ||
**Restrict Access:** | ||
- Use a role-based system (e.g., OpenZeppelin AccessControl) or simple ownership: | ||
```solidity | ||
import "@openzeppelin/contracts/access/Ownable.sol"; | ||
contract BulkPodYieldProcess is Ownable { | ||
function bulkProcessPendingYield(IDecentralizedIndex[] memory _idx) external onlyOwner { | ||
// ... | ||
} | ||
} | ||
``` |
Oops, something went wrong.