aloe-ii-preaudit's People
Forkers
dashbaord202401aloe-ii-preaudit's Issues
ERC20 Incompatibility: Lender is not protected for the ERC20 approval race condition
Summary
This is a legitimate known issue for the ERC20 standard implementation ERC20-issue
Vulnerability Detail
Lender is not protected for the ERC20 approval race condition
Impact
The scenario for exploitation is as follows:
Alice calls approve(Bob, 1000), allocating 1000 tokens for Bob to spend
Alice opts to change the amount approved for Bob to spend to a lesser amount via approve(Bob, 500). Once mined, this decreases the number of tokens that Bob can spend to 500.
Bob sees the transaction and calls transferFrom(Alice, X, 1000) before approve(Bob, 500) has been mined.
If Bob’s transaction is mined prior to Alice’s, 1000 tokens will be transferred by Bob.
However, once Alice’s transaction is mined, Bob can call transferFrom(Alice, X, 500), transferring a total of 1500 tokens despite Alice attempting to limit the total token allowance to 500.
Code Snippet
Tool used
Slither
Recommendation
consider using the safeIncreaseAllowance()
/ safeDecreaseAllowance()
/ forceApproval()
pattern from OpenZeppelin/SafeERC20
if gas usage is a concern, similar functionality can be achieved by using an approveOnlyIfBelowThreshold()
|| approveOnlyIfApprovalZero()
pattern, in which approvals are only set if the approval falls below a predefined threshold, or zero.
(src/Borrower.sol#194-286) Cross functional reentrancy risk
Summary
Reentrancy in Borrower.liquidate(ILiquidator,bytes,uint256,uint40) (src/Borrower.sol#194-286):
External calls:
- assets = _getAssets(slot0_,prices,true) (src/Borrower.sol#209)
- (burned0,burned1) = UNISWAP_POOL.burn(lower,upper,liquidity) (src/Borrower.sol#541)
- (collected0,collected1) = UNISWAP_POOL.collect(recipient,lower,upper,type()(uint128).max,type()(uint128).max) (src/Borrower.sol#542)
- callee.swap1For0(data,available1,liabilities0) (src/Borrower.sol#264)
- callee.swap0For1(data,available0,liabilities1) (src/Borrower.sol#274)
- _repay(repayable0,repayable1) (src/Borrower.sol#280)
- LENDER0.repay(amount0,address(this)) (src/Borrower.sol#548)
- LENDER1.repay(amount1,address(this)) (src/Borrower.sol#552)
State variables written after the call(s):
- slot0 = (slot0_ & SLOT0_MASK_POSITIONS) | SLOT0_DIRT (src/Borrower.sol#281)
Borrower.slot0 (src/Borrower.sol#114) can be used in cross function reentrancies:
- Borrower.getUniswapPositions() (src/Borrower.sol#457-459)
- Borrower.liquidate(ILiquidator,bytes,uint256,uint40) (src/Borrower.sol#194-286)
- Borrower.modify(IManager,bytes,uint40) (src/Borrower.sol#299-327)
- Borrower.onlyInModifyCallback() (src/Borrower.sol#116-119)
- Borrower.slot0 (src/Borrower.sol#114)
- Borrower.warn(uint40) (src/Borrower.sol#155-173)
Vulnerability Detail
slot0
is being written after external calls which is not recommended.
Code Snippet
if (shouldSwap) {
uint256 unleashTime = (slot0_ & SLOT0_MASK_UNLEASH) >> 208;
require(0 < unleashTime && unleashTime < block.timestamp, "Aloe: grace");
incentive1 /= strain;
if (liabilities0 > 0) {
// NOTE: This value is not constrained to `TOKEN1.balanceOf(address(this))`, so liquidators
// are responsible for setting `strain` such that the transfer doesn't revert. This shouldn't
// be an issue unless the borrower has already started accruing bad debt.
uint256 available1 = mulDiv128(liabilities0, priceX128) + incentive1;
TOKEN1.safeTransfer(address(callee), available1);
callee.swap1For0(data, available1, liabilities0);
repayable0 += liabilities0;
} else {
// NOTE: This value is not constrained to `TOKEN0.balanceOf(address(this))`, so liquidators
// are responsible for setting `strain` such that the transfer doesn't revert. This shouldn't
// be an issue unless the borrower has already started accruing bad debt.
uint256 available0 = Math.mulDiv(liabilities1 + incentive1, Q128, priceX128);
TOKEN0.safeTransfer(address(callee), available0);
callee.swap0For1(data, available0, liabilities1);
repayable1 += liabilities1;
}
}
**_repay(repayable0, repayable1);
slot0 = (slot0_ & SLOT0_MASK_POSITIONS) | SLOT0_DIRT;**
Tool used
Slither
Recommendation
because slot0
is not used after line 253, we can reset slot0
safely before our external calls like this:
if (shouldSwap) {
uint256 unleashTime = (slot0_ & SLOT0_MASK_UNLEASH) >> 208;
require(0 < unleashTime && unleashTime < block.timestamp, "Aloe: grace");
slot0 = (slot0_ & SLOT0_MASK_POSITIONS) | SLOT0_DIRT;
//state modifying code
........
_repay(repayable0, repayable1);
} else {
slot0 = (slot0_ & SLOT0_MASK_POSITIONS) | SLOT0_DIRT;
//state modifying code
_repay(repayable0, repayable1);
}
this way, slot0
is written to before any external calls to tokens
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.