Platform: Code4rena
Start Date: 04/01/2022
Pot Size: $75,000 USDC
Total HM: 17
Participants: 33
Period: 7 days
Judge: 0xean
Total Solo HM: 14
Id: 74
League: ETH
Rank: 1/33
Findings: 7
Award: $24,145.68
🌟 Selected for report: 7
🚀 Solo Findings: 4
🌟 Selected for report: jayjonah8
5679.4846 USDC - $5,679.48
jayjonah8
In TimeswapPair.sol, the mint() function has a callback in the middle of the function while there are still updates to state that take place after the callback. The lock modifier guards against reentrancy but not against cross function reentrancy. Since the protocol implements Uniswap like functionality, this can be extremely dangerous especially with regard to composability/interacting with other protocols and contracts. The callback before important state changes (updates to reserve asset, collateral, and totalDebtCreated) also violates the Checks Effects Interactions best practices further widening the attack surface.
https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html
cross function reentrancy https://medium.com/coinmonks/protect-your-solidity-smart-contracts-from-reentrancy-attacks-9972c3af7c21
Manual code review
The callback Callback.mint(asset, collateral, xIncrease, dueOut.collateral, data) should be placed at the end of the mint() function after all state updates have taken place.
#0 - Mathepreneur
2022-01-18T10:02:16Z
🌟 Selected for report: jayjonah8
5679.4846 USDC - $5,679.48
jayjonah8
In TimeswapPair.sol, the lend() function has a callback to the msg.sender in the middle of the function while there are still updates to state that take place after the callback. The lock modifier guards against reentrancy but not against cross function reentrancy. Since the protocol implements Uniswap like functionality, this can be extremely dangerous especially with regard to composability/interacting with other protocols and contracts. The callback before important state changes (updates to totalClaims bonds, insurance and reserves assets) also violates the Checks Effects Interactions best practices further widening the attack surface.
https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html
cross function reentrancy https://medium.com/coinmonks/protect-your-solidity-smart-contracts-from-reentrancy-attacks-9972c3af7c21
Manual code review
The callback Callback.lend(asset, xIncrease, data); should be placed at the end of the lend() function after all state updates have taken place.
#0 - Mathepreneur
2022-01-18T10:00:12Z
🌟 Selected for report: jayjonah8
5679.4846 USDC - $5,679.48
jayjonah8
In TimeswapPair.sol, the borrow() function has a callback to the msg.sender in the middle of the function while there are still updates to state that take place after the callback. The lock modifier guards against reentrancy but not against cross function reentrancy. Since the protocol implements Uniswap like functionality, this can be extremely dangerous especially with regard to composability/interacting with other protocols and contracts. The callback before important state changes (updates to collateral, totalDebtCreated and reserves assets) also violates the Checks Effects Interactions best practices further widening the attack surface.
https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html
cross function reentrancy https://medium.com/coinmonks/protect-your-solidity-smart-contracts-from-reentrancy-attacks-9972c3af7c21
Manual code review
The callback Callback.borrow(collateral, dueOut.collateral, data); should be placed at the end of the borrow() function after all state updates have taken place.
#0 - Mathepreneur
2022-01-18T09:58:41Z
🌟 Selected for report: jayjonah8
5679.4846 USDC - $5,679.48
jayjonah8
In TimeswapPair.sol, the pay() function has a callback to the msg.sender in the middle of the function while there are still updates to state that take place after the callback. The lock modifier guards against reentrancy but not against cross function reentrancy. Since the protocol implements Uniswap like functionality, this can be extremely dangerous especially with regard to composability/interacting with other protocols and contracts. The callback before important state changes (updates to reserves collateral and reserves assets) also violates the Checks Effects Interactions best practices further widening the attack surface.
https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html
cross function reentrancy https://medium.com/coinmonks/protect-your-solidity-smart-contracts-from-reentrancy-attacks-9972c3af7c21
Manual code review
The callback "if (assetIn > 0) Callback.pay(asset, assetIn, data);" should be placed at the end of the pay() function after all state updates have taken place.
#0 - Mathepreneur
2022-01-18T09:56:48Z
766.7304 USDC - $766.73
jayjonah8
In CollateralizedDebt.sol, the mint() function calls _safeMint() which has a callback to the "to" address argument. Functions with callbacks should have reentrancy guards in place for protection against possible malicious actors both from inside and outside the protocol.
Manual code review
Add a reentrancy guard modifier on the mint() function in CollateralizedDebt.sol
🌟 Selected for report: jayjonah8
567.9485 USDC - $567.95
jayjonah8
In TimeswapConvenience.sol the constructor takes in 2 addresses for _factory and _weth and sets them in storage without checking that they are unique which can introduce possible costly errors during deployment.
Manuel code review
Add to TimeswapConvenience.sol constructor: require(_factory != _weth, "Duplicate address")
#0 - Mathepreneur
2022-01-17T20:26:58Z
🌟 Selected for report: jayjonah8
93.0809 USDC - $93.08
jayjonah8
In TimeswapPair.sol the pay() function calls safeTransfer() and does so using the SafeTransfer.sol library when it can simply add the open zeppelin SafeERC20.sol import directly inside TimeswapPair.sol itself eliminating the unnecessary code in the protocols own SafeTransfer library.
Manual code review
Use the open zeppelin SafeERC20 import directly inside the TimeswapPair.sol file instead of calling your own library. The extra safeTransfer library can then be deleted.
#0 - Mathepreneur
2022-01-18T09:52:24Z