-
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
6317751
commit 716f718
Showing
1,068 changed files
with
87,408 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,68 @@ | ||
Clever Fern Rooster | ||
|
||
Medium | ||
|
||
# `contractBurnFrom` function miss the `onlyCoreContracts` modifier | ||
|
||
### Summary | ||
|
||
The burn of the tokens is only reserved to the core contracts: | ||
```solidity | ||
function burn(uint256 value) public override onlyCoreContracts { | ||
super.burn(value); | ||
} | ||
function burnFrom( | ||
address account, | ||
uint256 value | ||
) public override onlyCoreContracts { | ||
super.burnFrom(account, value); | ||
} | ||
``` | ||
|
||
Any user could bypass this by burning tokens with [`contractBurnFrom`](https://github.com/sherlock-audit/2024-11-autonomint/blob/main/Blockchain/Blockchian/contracts/Token/USDa.sol#L167-L170) that miss the `onlyCoreContracts` modifier: | ||
```solidity | ||
function contractBurnFrom( | ||
address owner, | ||
uint256 amount | ||
) public returns (bool) { | ||
uint256 currentAllowance = contractAllowances[owner][msg.sender]; | ||
if (currentAllowance != type(uint256).max) { | ||
if (currentAllowance < amount) revert ERC20InsufficientAllowance(msg.sender, currentAllowance, amount); | ||
_contractApprove(owner, msg.sender, currentAllowance - amount); | ||
_burn(owner, amount); | ||
return true; | ||
} else { | ||
return false; | ||
} | ||
} | ||
``` | ||
|
||
### Root Cause | ||
|
||
Missing access control on the `contractBurnFrom` function. | ||
|
||
### Internal pre-conditions | ||
|
||
_No response_ | ||
|
||
### External pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
1. Address A gives `x` allowance to address B. | ||
2. Address B call `contractBurnFrom` to burn `x` tokens of address A. | ||
|
||
### Impact | ||
|
||
Breaks core contract functionality. | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
Add the `onlyCoreContracts` modifier to the `contractBurnFrom` function. |
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,59 @@ | ||
Exotic Navy Elephant | ||
|
||
Medium | ||
|
||
# Chainlink Oracle returns stale price due to missing checks on heartbeat | ||
|
||
### Summary | ||
|
||
Chainlink price feeds usually update the price of an asset once it deviates a certain percentage. For example the ETH/USD price feed updates on 0.5% change of price. If there is no change for 1 hour, the price feed updates again - this is called heartbeat: <https://data.chain.link/feeds/ethereum/mainnet/eth-usd>. | ||
|
||
This was not implemented to prevent stale prices. | ||
|
||
### Root Cause | ||
|
||
The implementation does not check for stale prices. | ||
```solidity | ||
(, int256 price_, , , ) = oracle.latestRoundData(); //@audit | ||
// If the token is ETH | ||
if (underlying == assetAddress[IBorrowing.AssetName.ETH]) { | ||
// Return Exchange rate as 1 and ETH price with 2 decimals | ||
return (1 ether, uint128((uint256(price_) / 1e6))); | ||
} else { | ||
(, uint128 ethPrice) = _price(assetAddress[IBorrowing.AssetName.ETH]); | ||
// Return Exchange rate and ETH price with 2 decimals | ||
return (uint128(uint256(price_)), ethPrice); | ||
} | ||
``` | ||
https://github.com/sherlock-audit/2024-11-autonomint/blob/0d324e04d4c0ca306e1ae4d4c65f0cb9d681751b/Blockchain/Blockchian/contracts/oracles/MasterPriceOracle.sol#L83C1-L92C14 | ||
### Internal pre-conditions | ||
|
||
_No response_ | ||
|
||
### External pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
_No response_ | ||
|
||
### Impact | ||
|
||
Outdated prices are returned providing inaccurate old prices to users. | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
Introduce a new parameter that could be passed alongside the oracle which refers to the heartbeat of that oracle, so that `updatedAt` could be compared with that value. | ||
E.g | ||
```diff | ||
+ ( , int price, , uint256 updatedAt, ) = oracle.latestRoundData(); | ||
+ require(price > 0, "Invalid price"); | ||
+ require(answeredInRound >= roundId, "Stale price data"); | ||
+ // Timestamp validation to ensure freshness | ||
+ require(block.timestamp - updatedAt <= MAX_DELAY, "Stale price data"); //MAX_DELAY specify for each address | ||
``` |
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,43 @@ | ||
Flat Macaroon Alligator | ||
|
||
High | ||
|
||
# USDT does not return any value on transfer, which means that all transfer and transferFrom operations will fail. | ||
|
||
### Summary | ||
|
||
The protocol uses the USDT token as one of its main tokens, as stated in the README. However, the problem lies in how transfers are implemented for this token. USDT is an unusual token that does not return any value after `transferFrom`. The protocol checks if the return value is `true`, which means this condition will fail 100% of the time. | ||
|
||
### Root Cause | ||
|
||
The USDT token implementation does not return a boolean value after a transfer because it does not follow certain default ERC-20 guidelines. This means that any attempt to check the returned value will result in false. | ||
|
||
Instances: | ||
|
||
https://github.com/sherlock-audit/2024-11-autonomint/blob/main/Blockchain/Blockchian/contracts/lib/CDSLib.sol#L408-L412 | ||
https://github.com/sherlock-audit/2024-11-autonomint/blob/main/Blockchain/Blockchian/contracts/lib/CDSLib.sol#L553-L558 | ||
https://github.com/sherlock-audit/2024-11-autonomint/blob/main/Blockchain/Blockchian/contracts/lib/CDSLib.sol#L408-L413 | ||
|
||
### Internal pre-conditions | ||
|
||
Any USDT transfer made by using transferFrom/transfer. | ||
|
||
### External pre-conditions | ||
|
||
_No response_ | ||
|
||
### Attack Path | ||
|
||
_No response_ | ||
|
||
### Impact | ||
|
||
All instances where plain transfer or transferFrom are used for USDT will fail, blocking execution every time USDT is transferred. | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
Use OpenZeppelin's `safeTransfer` implementation instead. |
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,42 @@ | ||
Flat Macaroon Alligator | ||
|
||
Medium | ||
|
||
# Chainlink's oracle is used incorrectly, resulting in inaccurate prices. | ||
|
||
### Summary | ||
|
||
The protocol supports two oracles: Redstone and Chainlink. Chainlink is only supported on the Optimism chain, and the issue lies in price validation, as there is no validation for either the received price or its staleness. | ||
|
||
### Root Cause | ||
|
||
As shown here: [[MasterPriceOracle.sol#L80-L92](https://github.com/sherlock-audit/2024-11-autonomint/blob/main/Blockchain/Blockchian/contracts/oracles/MasterPriceOracle.sol#L80-L92)](https://github.com/sherlock-audit/2024-11-autonomint/blob/main/Blockchain/Blockchian/contracts/oracles/MasterPriceOracle.sol#L80-L92), the price is retrieved from the Chainlink aggregator contract. However, there is no validation to ensure that the price is greater than 0 or that it is not stale (no `updatedAt` validation). | ||
|
||
### Internal pre-conditions | ||
|
||
Contract deployed on Optimism chain. | ||
|
||
### External pre-conditions | ||
|
||
Chainlink returns incorrect/stale price. | ||
|
||
### Attack Path | ||
|
||
_No response_ | ||
|
||
### Impact | ||
|
||
Protocol would use incorrect or stale price opening big vector of attack that comes with market price/provided price discrepancy. | ||
|
||
### PoC | ||
|
||
_No response_ | ||
|
||
### Mitigation | ||
|
||
Add default validation for the price to ensure that: | ||
|
||
- The price is not 0. | ||
- The price is not stale. | ||
|
||
Documentation: [Chainlink Data Feeds - Check the Timestamp of the Latest Answer](https://docs.chain.link/data-feeds#check-the-timestamp-of-the-latest-answer) |
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,90 @@ | ||
Low Tangerine Cod | ||
|
||
High | ||
|
||
# users will get more funds than suppose due to incorrect strike price logic | ||
|
||
### Summary | ||
|
||
calculateStrikePriceGains uses current eth price which is incorrect | ||
|
||
### Root Cause | ||
|
||
strike price - fixed price at which the holder of an options contract can buy (in the case of a call option) or sell (in the case of a put option) the underlying asset when the option is exercised. | ||
|
||
```solidity | ||
function calculateStrikePriceGains( | ||
uint128 depositedAmount, | ||
uint128 strikePrice, | ||
uint64 ethPrice | ||
) external view onlyBorrowingContract returns (uint128) { | ||
// Check the input params are non zero | ||
require(depositedAmount != 0 && strikePrice != 0 && ethPrice != 0,"Zero inputs in options"); | ||
uint64 currentEthPrice = ethPrice; | ||
// Calculate current deposited ETH value in USD | ||
uint128 currentEthValue = depositedAmount * currentEthPrice; | ||
uint128 ethToReturn; | ||
// If the current value is greater, then user will get eth | ||
if (currentEthValue > strikePrice) { | ||
ethToReturn = (currentEthValue - strikePrice) / currentEthPrice; | ||
} else { | ||
ethToReturn = 0; | ||
} | ||
return ethToReturn; | ||
} | ||
``` | ||
[Blockchain/Blockchian/contracts/Core_logic/Options.sol#L77](https://github.com/sherlock-audit/2024-11-autonomint/blob/main/Blockchain/Blockchian/contracts/Core_logic/Options.sol#L77) | ||
```solidity | ||
uint128 depositedAmountvalue = (params.depositDetail.depositedAmountInETH * params.depositDetail.ethPriceAtDeposit) / params.ethPrice; | ||
collateralToReturn = (depositedAmountvalue + params.options.calculateStrikePriceGains( | ||
params.depositDetail.depositedAmountInETH, | ||
params.depositDetail.strikePrice, | ||
params.ethPrice | ||
) | ||
); | ||
``` | ||
[](https://github.com/sherlock-audit/2024-11-autonomint/blob/main/Blockchain/Blockchian/contracts/lib/BorrowLib.sol#L483) | ||
|
||
Lets calculate: | ||
deposit 1ETH, ethprice 1000$, strike price = 1200 | ||
later: ethprice - 2000$. | ||
depositedAmountvalue = 0.5ETH | ||
calculateStrikePriceGains = (1 ether*2000- 1ether*1200)/2000 = 0.4ETH | ||
collateralToReturn = depositedAmountvalue+calculateStrikePriceGains = 0.9ETH. | ||
|
||
But calculate using strike price definition: | ||
(1ether * 1200/2000) ~ 0.6ETH | ||
|
||
so, protocol returning to user more than it should | ||
|
||
### Internal pre-conditions | ||
|
||
### External pre-conditions | ||
|
||
### Attack Path | ||
|
||
always happening | ||
|
||
### Impact | ||
|
||
Users withdraw more funds than the protocol expects them to have. | ||
|
||
### PoC | ||
|
||
### Mitigation | ||
calculateStrikePriceGains calculates amount to **remain** in protocol and the rest goes to user | ||
```diff | ||
- collateralToReturn = (depositedAmountvalue + params.options.calculateStrikePriceGains( | ||
- params.depositDetail.depositedAmountInETH, | ||
- params.depositDetail.strikePrice, | ||
- params.ethPrice | ||
- ) | ||
- ); | ||
+ collateralRemainingInWithdraw = (params.options.calculateStrikePriceGains( | ||
+ params.depositDetail.depositedAmountInETH, | ||
+ params.depositDetail.strikePrice, | ||
+ params.ethPrice | ||
+ )); | ||
+ collateralToReturn = params.depositDetail.depositedAmountInETH - collateralRemainingInWithdraw | ||
``` |
Oops, something went wrong.