Platform: Code4rena
Start Date: 19/01/2024
Pot Size: $36,500 USDC
Total HM: 9
Participants: 113
Period: 3 days
Judge: 0xsomeone
Id: 322
League: ETH
Rank: 26/113
Findings: 2
Award: $192.08
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: NPCsCorp
Also found by: 0x11singh99, 0xAadi, 0xBugSlayer, 0xE1, 0xPluto, 0xSimeon, 0xSmartContract, 0xabhay, 0xdice91, 0xprinc, Aamir, Aymen0909, CDSecurity, DadeKuma, DarkTower, EV_om, Eeyore, GeekyLumberjack, GhK3Ndf, Giorgio, Greed, Inference, JanuaryPersimmon2024, Kaysoft, Krace, Matue, MrPotatoMagic, NentoR, Nikki, PUSH0, Soliditors, Tendency, Tigerfrake, Timeless, Timenov, ZanyBonzy, ZdravkoHr, abiih, adeolu, al88nsk, azanux, bareli, boredpukar, cu5t0mpeo, d4r3d3v1l, darksnow, deth, dutra, ether_sky, haxatron, ke1caM, kodyvim, m4ttm, mgf15, mrudenko, nmirchev8, nobody2018, nuthan2x, peanuts, piyushshukla, ravikiranweb3, rouhsamad, seraviz, simplor, slylandro_star, stealth, th13vn, vnavascues, wangxx2026, zaevlad
0.1172 USDC - $0.12
DcntEth::setRouter
function has no access control. In this function router
state variable is set. After that router
is responsible to mint and burn DcntEth
tokens by calling DcntEth::mint
and DcntEth::burn
functions. Since anyone can set router using setRouter
function so attacker can call setRouter
function by passing his address and can become router
. When attacker's address will be set as router
,then he can call mint
or burn
functions of DcntEth.sol
,since these functions are protected by onlyRouter
modifier and attacker have become router
already using setRouter
. So he can mint/burn his desired quantity of DcntEth
tokens whatever he want.
We can see in below code snippet there is no access control on setRouter
function. So anyone can call this and set his address into router
state variable. After becoming router
he can call mint
and burn
easily with his desired quantity or unlimited quantity of DcntEth
tokens can be minted to attacker's passed address.
//@audit-issue Missing access control 20: function setRouter(address _router) public { 21: router = _router; 22: } //@audit attacker can call this function by becoming router 24: function mint(address _to, uint256 _amount) public onlyRouter { 25: _mint(_to, _amount); } //@audit attacker can call this function by becoming router 28: function burn(address _from, uint256 _amount) public onlyRouter { 29: _burn(_from, _amount); 30: }
onlyRouter Modifier src/DcntEth.sol#L8-L11
8: modifier onlyRouter() { 9: require(msg.sender == router); 10: _; 11: }
Attacker can become router
by passing his address in setRouter
function.
And he can mint
unlimited DcntEth
tokens to himself Or he can burn
all the DcntEth
tokens. By this the purpose of DcntEth
tokens will be totally destroyed. Attacker can take over all DcntEth
tokens.
Manual Review
Use any access control in the setRouter
function. So only allowed role can call this crucial function not everyone. Consider adding onlyOwner
modifier in this function since it is already used in other functions of this file mintByOwner
and burnByOwner
.
- 20: function setRouter(address _router) public { + 20: function setRouter(address _router) public onlyOwner { 21: router = _router; 22: }
Access Control
#0 - c4-pre-sort
2024-01-25T03:08:07Z
raymondfam marked the issue as sufficient quality report
#1 - c4-pre-sort
2024-01-25T03:08:14Z
raymondfam marked the issue as duplicate of #14
#2 - c4-judge
2024-02-03T13:10:26Z
alex-ppg marked the issue as satisfactory
🌟 Selected for report: c3phas
Also found by: 0x11singh99, Raihan, dharma09, hunter_w3b, slvDev
191.9635 USDC - $191.96
Note: It includes only those instances which were Missed by bot-report.
State
variables only set in the constructor
should be declared immutable
(Missed by bot)Note: It includes only those instances which were Missed by bot-report
Accessing state variables within a function involves an SLOAD operation, but immutable
variables can be accessed directly without the need of it, thus reducing gas costs. As these state variables are assigned only in the constructor, consider declaring them immutable
.
Avoids a Gsset(** 20000 gas**) in the constructor, and replaces the first access in each transaction(Gcoldsload - ** 2100 gas **) and each access thereafter(Gwarmacces - ** 100 gas ) with aPUSH32( 3 gas **).
4 instances
File : src/DecentBridgeExecutor.sol 09: IWETH weth; 10: bool public gasCurrencyIsEth;
File : src/bridge_adapters/DecentBridgeAdapter.sol 19: address bridgeToken;
File : src/DecentEthRouter.sol 15: IDecentBridgeExecutor public executor;
wrapped
variable address in constructor
instead of function and make it immutable
wrapped
is referring to ERC20 version of native currency of that chain. like WETH for Ethereum Mainnet.
So they are one per chain and fixed contracts on every chain. So they can be marked immutable since these contracts using them will be deployed on different chains separately.
Avoids a Gsset(20000 gas) in the constructor, and replaces the first access in each transaction(Gcoldsload - 2100 gas) and each access thereafter(Gwarmacces - 100 gas ) with aPUSH32( 3 gas)
2 instances
address wrapped
in the constructor and make it immutable and remove the setter function setWrapped
File : src/swappers/UniSwapper.sol constructor() UTBOwned() {} ... address payable public wrapped; ... function setWrapped(address payable _wrapped) public onlyOwner { wrapped = _wrapped; }
File : src/swappers/UniSwapper.sol - constructor() UTBOwned() {} uint8 public constant SWAPPER_ID = 0; address public uniswap_router; - address payable public wrapped; + address payable public immutable wrapped; + constructor(address payable _wrapped) UTBOwned() { + wrapped = _wrapped; + } function setRouter(address _router) public onlyOwner { uniswap_router = _router; } - function setWrapped(address payable _wrapped) public onlyOwner { - wrapped = _wrapped; - }
wrapped
in the constructor and make it immutable and remove the setter function setWrapped
File : src/UTB.sol constructor() Owned(msg.sender) {} ... IWETH wrapped; ... function setWrapped(address payable _wrapped) public onlyOwner { wrapped = IWETH(_wrapped); }
File : src/UTB.sol - constructor() Owned(msg.sender) {} IUTBExecutor executor; IUTBFeeCollector feeCollector; - IWETH wrapped; + IWETH immutable wrapped; mapping(uint8 => address) public swappers; mapping(uint8 => address) public bridgeAdapters; + constructor(address payable _wrapped) Owned(msg.sender) { + wrapped = IWETH(_wrapped); + } /** * @dev Sets the executor. * @param _executor The address of the executor. */ function setExecutor(address _executor) public onlyOwner { executor = IUTBExecutor(_executor); } /** * @dev Sets the wrapped native token. * @param _wrapped The address of the wrapped token. */ - function setWrapped(address payable _wrapped) public onlyOwner { - wrapped = IWETH(_wrapped); - }
If you want to avoid magic numbers then use constant
1 instance
File : src/DecentEthRouter.sol 96: uint256 GAS_FOR_RELAY = 100000; 97: uint256 gasAmount = GAS_FOR_RELAY + _dstGasForCall;
modifier
2 instances
onlyIfWeHaveEnoughReserves(amount)
modifier checkonlyIfWeHaveEnoughReserves
modifier is checking that our DecentEthRouter
contract have enough weth or not. Since It will be checked in Weth contract also when this DecentEthRouter
will call withdraw or transfer function in weth contract. And when this caller contract don't have enough reserves WETH
contract will revert automatically which will revert entire transaction. So there is no need to add extra modifier check here in our contract checking same thing.
It will save gas for not using that modifier and deleting this modifier will also some save deployment gas also.
It will save the gas every time redeemEth
function called.
File : src/DecentEthRouter.sol 285: function redeemEth( 286: uint256 amount 287: ) public onlyIfWeHaveEnoughReserves(amount) { 288: dcntEth.transferFrom(msg.sender, address(this), amount); 289: weth.withdraw(amount); 290: payable(msg.sender).transfer(amount); 291: }
File : src/DecentEthRouter.sol function redeemEth( uint256 amount - ) public onlyIfWeHaveEnoughReserves(amount) { + ) public { + weth.withdraw(amount); dcntEth.transferFrom(msg.sender, address(this), amount); - weth.withdraw(amount); payable(msg.sender).transfer(amount); }
It will save the gas every time redeemWeth
function called.
File : src/DecentEthRouter.sol 294: function redeemWeth( 295: uint256 amount 296: ) public onlyIfWeHaveEnoughReserves(amount) { 297: dcntEth.transferFrom(msg.sender, address(this), amount); 298: weth.transfer(msg.sender, amount); 299: }
File : src/DecentEthRouter.sol function redeemWeth( uint256 amount - ) public onlyIfWeHaveEnoughReserves(amount) { + ) public { + weth.transfer(msg.sender, amount); dcntEth.transferFrom(msg.sender, address(this), amount); - weth.transfer(msg.sender, amount); }
payable
to save gasNote: It includes only those instances which were Missed by bot-report
If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost
6 instances
File : src/DcntEth.sol 24: function mint(address _to, uint256 _amount) public onlyRouter { 28: function burn(address _from, uint256 _amount) public onlyRouter { 32: function mintByOwner(address _to, uint256 _amount) public onlyOwner { 36: function burnByOwner(address _from, uint256 _amount) public onlyOwner {
File : src/UTBFeeCollector.sol 18: function setSigner(address _signer) public onlyOwner { 69: function redeemFees(address token, uint amount) public onlyOwner {
If a value is only intended to be used once then it should not be cached. Caching the value will result in unnecessary stack manipulation.
2 instances
ISwapper(swapper)
and IBridgeAdapter(bridge)
File : src/UTB.sol 326: ISwapper s = ISwapper(swapper); 327: swappers[s.getId()] = swapper; 335: IBridgeAdapter b = IBridgeAdapter(bridge); 336: bridgeAdapters[b.getId()] = bridge;
File : src/UTB.sol - ISwapper s = ISwapper(swapper); - swappers[s.getId()] = swapper; + swappers[ISwapper(swapper).getId()] = swapper; - IBridgeAdapter b = IBridgeAdapter(bridge); - bridgeAdapters[b.getId()] = bridge; + bridgeAdapters[IBridgeAdapter(bridge).getId()] = bridge;
calldata
instead of memory
Note: Missed by bot-report
When you specify a data location as memory, that value will be copied into memory. When you specify the location as calldata, the value will stay static within calldata. If the value is a large, complex type, using memory may result in extra memory expansion costs.
5 instances
File : src/DecentEthRouter.sol 113: function estimateSendAndCallFee( 114: uint8 msgType, 115: uint16 _dstChainId, 116: address _toAddress, 117: uint _amount, 118: uint64 _dstGasForCall, 119: bool deliverEth, 120: bytes memory payload //@audit use calldata 121: ) public view returns (uint nativeFee, uint zroFee) { 197: function bridgeWithPayload( 198: uint16 _dstChainId, 199: address _toAddress, 200: uint _amount, 201: bool deliverEth, 202: uint64 _dstGasForCall, 203: bytes memory additionalPayload //@audit use calldata 204: ) public payable { 237: function onOFTReceived( 238: uint16 _srcChainId, 239: bytes calldata, 240: uint64, 241: bytes32, 242: uint _amount, 243: bytes memory _payload //@audit use calldata 244: ) external override onlyLzApp {
File : src/UTBFeeCollector.sol 44: function collectFees( 45: FeeStructure calldata fees, 46: bytes memory packedInfo, //@audit use calldata 47: bytes memory signature //@audit use calldata 48: ) public payable onlyUtb {
Since these are external pure functions so neither they can be used in this contract nor they telling something about contract. So it is unnecessary to have external pure functions in contract purely for calculation purpose. If it is called by another contract than it will be better they do it in their contract. If it is called from outside then it will be better to do it there. Since they are not serving any purpose related to smart contract so they can be removed.
2 instances
File : src/bridge_adapters/StargateBridgeAdapter.sol 55: function getBridgeToken( 61: function getBridgedAmount(
Note: It includes only those instances which were Missed by bot-report
2 instances
File : src/UTBFeeCollector.sol //@audit `SwapInstructions` and `BridgeInstructions` 05: import {SwapInstructions, FeeStructure, BridgeInstructions} from "./CommonTypes.sol";
Note: It includes only those instances which were Missed by bot-report
6 instances
File : src/DcntEth.sol 24: function mint(address _to, uint256 _amount) public onlyRouter { 25: _mint(_to, _amount); //@audit check `_amount` for zero 26: } 27: 28: function burn(address _from, uint256 _amount) public onlyRouter { 29: _burn(_from, _amount); //@audit check `_amount` for zero 30: } 31: 32: function mintByOwner(address _to, uint256 _amount) public onlyOwner { 33: _mint(_to, _amount); //@audit check `_amount` for zero 34: } 35: 36: function burnByOwner(address _from, uint256 _amount) public onlyOwner { 37: _burn(_from, _amount); //@audit check `_amount` for zero 38: }
File : src/UTBFeeCollector.sol 70: if (token == address(0)) { 71: payable(owner).transfer(amount); //@audit check `amount` for zero 72: } else { 73: IERC20(token).transfer(owner, amount); //@audit check `amount` for zero 74: }
underflow
/overflow
should be used uncheckedNote: It includes only those instances which were Missed by bot-report
2 instances
File : src/DecentEthRouter.sol 61: uint256 balance = balanceOf[msg.sender]; 62: require(balance >= amount, "not enough balance"); 63: _; 64: balanceOf[msg.sender] -= amount; 65: }
File : src/DecentEthRouter.sol //@audit can be marked unchecked since first is `100000` and second is uint64 so no chance to overflow 97: uint256 gasAmount = GAS_FOR_RELAY + _dstGasForCall;
2 instances
File : src/DecentEthRouter.sol 61: uint256 balance = balanceOf[msg.sender]; 62: require(balance >= amount, "not enough balance"); 63: _; 64: balanceOf[msg.sender] -= amount; 65: }
File : src/DecentEthRouter.sol uint256 balance = balanceOf[msg.sender]; require(balance >= amount, "not enough balance"); _; - balanceOf[msg.sender] -= amount; + balanceOf[msg.sender] = balance - amount; }
Note: It includes only those instances which were Missed by bot-report
6 instances
weth
File : src/DecentBridgeExecutor.sol 30: uint256 balanceBefore = weth.balanceOf(address(this)); 31: weth.approve(target, amount); 32: 33: (bool success, ) = target.call(callPayload); 34: 35: if (!success) { 36: weth.transfer(from, amount); 37: return; 38: } 39: 40: uint256 remainingAfterCall = amount - 41: (balanceBefore - weth.balanceOf(address(this))); 42: 43: // refund the sender with excess WETH 44: weth.transfer(from, remainingAfterCall);
File : src/DecentBridgeExecutor.sol + IWETH _weth = weth; - uint256 balanceBefore = weth.balanceOf(address(this)); - weth.approve(target, amount); + uint256 balanceBefore = _weth.balanceOf(address(this)); + _weth.approve(target, amount); (bool success, ) = target.call(callPayload); if (!success) { - weth.transfer(from, amount); + _weth.transfer(from, amount); return; } uint256 remainingAfterCall = amount - - (balanceBefore - weth.balanceOf(address(this))); + (balanceBefore - _weth.balanceOf(address(this))); // refund the sender with excess WETH - weth.transfer(from, remainingAfterCall); + _weth.transfer(from, remainingAfterCall);
weth
File : src/DecentEthRouter.sol 266: if (weth.balanceOf(address(this)) < _amount) { 267: dcntEth.transfer(_to, _amount); 268: return; 269: } 270: 271: if (msgType == MT_ETH_TRANSFER) { 272: if (!gasCurrencyIsEth || !deliverEth) { 273: weth.transfer(_to, _amount); 274: } else { 275: weth.withdraw(_amount); 276: payable(_to).transfer(_amount); 277: } 278: } else { 289: weth.approve(address(executor), _amount);
File : src/DecentEthRouter.sol + IWETH _weth = weth; - if (weth.balanceOf(address(this)) < _amount) { + if (_weth.balanceOf(address(this)) < _amount) { dcntEth.transfer(_to, _amount); return; } if (msgType == MT_ETH_TRANSFER) { if (!gasCurrencyIsEth || !deliverEth) { - weth.transfer(_to, _amount); + _weth.transfer(_to, _amount); } else { - weth.withdraw(_amount); + _weth.withdraw(_amount); payable(_to).transfer(_amount); } } else { - weth.approve(address(executor), _amount); + _weth.approve(address(executor), _amount);
executor
File : src/DecentEthRouter.sol 279: weth.approve(address(executor), _amount); 280: executor.execute(_from, _to, deliverEth, _amount, callPayload);
File : src/DecentEthRouter.sol + IDecentBridgeExecutor _executor = executor; - weth.approve(address(executor), _amount); - executor.execute(_from, _to, deliverEth, _amount, callPayload); + weth.approve(address(_executor), _amount); + _executor.execute(_from, _to, deliverEth, _amount, callPayload);
wrapped
File : src/UTB.sol 74: if (swapParams.tokenIn == address(0)) { 75: require(msg.value >= swapParams.amountIn, "not enough native"); 76: wrapped.deposit{value: swapParams.amountIn}(); 77: swapParams.tokenIn = address(wrapped); 78: swapInstructions.swapPayload = swapper.updateSwapParams( 79: swapParams, 80: swapInstructions.swapPayload 81: ); 82: } else if (retrieveTokenIn) { 83: IERC20(swapParams.tokenIn).transferFrom( 84: msg.sender, 85: address(this), 86: swapParams.amountIn 87: ); 88: } 89: 90: IERC20(swapParams.tokenIn).approve( 91: address(swapper), 92: swapParams.amountIn 93: ); 94: 95: (tokenOut, amountOut) = swapper.swap(swapInstructions.swapPayload); 96: 97: if (tokenOut == address(0)) { 98: wrapped.withdraw(amountOut);
File : src/UTB.sol + IWETH _wrapped = wrapped; if (swapParams.tokenIn == address(0)) { require(msg.value >= swapParams.amountIn, "not enough native"); - wrapped.deposit{value: swapParams.amountIn}(); - swapParams.tokenIn = address(wrapped); + _wrapped.deposit{value: swapParams.amountIn}(); + swapParams.tokenIn = address(_wrapped); swapInstructions.swapPayload = swapper.updateSwapParams( swapParams, swapInstructions.swapPayload ); } else if (retrieveTokenIn) { IERC20(swapParams.tokenIn).transferFrom( msg.sender, address(this), swapParams.amountIn ); } IERC20(swapParams.tokenIn).approve( address(swapper), swapParams.amountIn ); (tokenOut, amountOut) = swapper.swap(swapInstructions.swapPayload); if (tokenOut == address(0)) { - wrapped.withdraw(amountOut); + _wrapped.withdraw(amountOut);
executor
File : src/UTB.sol 152: IERC20(tokenOut).approve(address(executor), amountOut); 153: executor.execute(
File : src/UTB.sol + IUTBExecutor _executor = executor; - IERC20(tokenOut).approve(address(executor), amountOut); - executor.execute( + IERC20(tokenOut).approve(address(_executor), amountOut); + _executor.execute(
feeCollector
File : src/UTB.sol 233: if (address(feeCollector) != address(0)) { 234: uint value = 0; 235: if (fees.feeToken != address(0)) { 236: IERC20(fees.feeToken).transferFrom( 237: msg.sender, 238: address(this), 239: fees.feeAmount 240: ); 241: IERC20(fees.feeToken).approve( 242: address(feeCollector), 243: fees.feeAmount 244: ); 245: } else { 246: value = fees.feeAmount; 247: } 248: feeCollector.collectFees{value: value}(fees, packedInfo, signature);
File : src/UTB.sol + IUTBFeeCollector _feeCollector = feeCollector; - if (address(feeCollector) != address(0)) { + if (address(_feeCollector) != address(0)) { uint value = 0; if (fees.feeToken != address(0)) { IERC20(fees.feeToken).transferFrom( msg.sender, address(this), fees.feeAmount ); IERC20(fees.feeToken).approve( - address(feeCollector), + address(_feeCollector), fees.feeAmount ); } else { value = fees.feeAmount; } - feeCollector.collectFees{value: value}(fees, packedInfo, signature); + _feeCollector.collectFees{value: value}(fees, packedInfo, signature);
address(this)
instead of re-calculateCalculating address(this)
takes some gas so it is better to cache it instead of re-calculating multiple times in same function.
2 instances
File : src/DecentBridgeExecutor.sol 30: uint256 balanceBefore = weth.balanceOf(address(this)); 31: weth.approve(target, amount); 32: 33: (bool success, ) = target.call(callPayload); 34: 35: if (!success) { 36: weth.transfer(from, amount); 37: return; 38: } 39: 40: uint256 remainingAfterCall = amount - 41: (balanceBefore - weth.balanceOf(address(this)));
File : src/DecentBridgeExecutor.sol + address _this = address(this); - uint256 balanceBefore = weth.balanceOf(address(this)); + uint256 balanceBefore = weth.balanceOf(_this); weth.approve(target, amount); (bool success, ) = target.call(callPayload); if (!success) { weth.transfer(from, amount); return; } uint256 remainingAfterCall = amount - - (balanceBefore - weth.balanceOf(address(this))); + (balanceBefore - weth.balanceOf(_this));
File : src/UTBExecutor.sol 59: uint initBalance = IERC20(token).balanceOf(address(this)); 60: 61: IERC20(token).transferFrom(msg.sender, address(this), amount); 62: IERC20(token).approve(paymentOperator, amount); 63: 64: if (extraNative > 0) { 65: (success, ) = target.call{value: extraNative}(payload); 66: if (!success) { 67: (refund.call{value: extraNative}("")); 68: } 69: } else { 70: (success, ) = target.call(payload); 71: } 72: 73: uint remainingBalance = IERC20(token).balanceOf(address(this)) - 74: initBalance;
File : src/UTBExecutor.sol + address _this = address(this); - uint initBalance = IERC20(token).balanceOf(address(this)); + uint initBalance = IERC20(token).balanceOf(_this); - IERC20(token).transferFrom(msg.sender, address(this), amount); + IERC20(token).transferFrom(msg.sender, _this, amount); IERC20(token).approve(paymentOperator, amount); if (extraNative > 0) { (success, ) = target.call{value: extraNative}(payload); if (!success) { (refund.call{value: extraNative}("")); } } else { (success, ) = target.call(payload); } - uint remainingBalance = IERC20(token).balanceOf(address(this)) - + uint remainingBalance = IERC20(token).balanceOf(_this) - initBalance;
if-else
statement to save gasSince it is better to write less gas consuming condition before more gas consumer. In if statement due to operator short-circuiting. First is evaluated if 2nd is not needed then 2nd will not be evaluated in || OR && operations.
To save 2 not operations and 1 Sload if deliveryEth is false we can refactor this below code.
2 instances
File : src/DecentBridgeExecutor.sol 77: if (!gasCurrencyIsEth || !deliverEth) { 78: _executeWeth(from, target, amount, callPayload); 79: } else { 80: _executeEth(from, target, amount, callPayload); 81: }
File : src/DecentBridgeExecutor.sol - if (!gasCurrencyIsEth || !deliverEth) { - _executeWeth(from, target, amount, callPayload); - } else { - _executeEth(from, target, amount, callPayload); - } + if (deliverEth && gasCurrencyIsEth) { + _executeEth(from, target, amount, callPayload); + } else { + _executeWeth(from, target, amount, callPayload); + }
File : src/DecentEthRouter.sol 272: if (!gasCurrencyIsEth || !deliverEth) { 273: weth.transfer(_to, _amount); 274: } else { 275: weth.withdraw(_amount); 276: payable(_to).transfer(_amount); 277: }
File : src/DecentEthRouter.sol - if (!gasCurrencyIsEth || !deliverEth) { - weth.transfer(_to, _amount); - } else { - weth.withdraw(_amount); - payable(_to).transfer(_amount); - } + if (deliverEth && gasCurrencyIsEth) { + weth.withdraw(_amount); + payable(_to).transfer(_amount); + } else { + weth.transfer(_to, _amount); + }
onOFTReceived
to save gasWe can refactor this function to avoid 1 abi.decode. It will have same impact. 1 instance
File : src/DecentEthRouter.sol 245: (uint8 msgType, address _from, address _to, bool deliverEth) = abi 246: .decode(_payload, (uint8, address, address, bool)); 247: 248: bytes memory callPayload = ""; 249: 250: if (msgType == MT_ETH_TRANSFER_WITH_PAYLOAD) { 251: (, , , , callPayload) = abi.decode( 252: _payload, 253: (uint8, address, address, bool, bytes) 254: );
File : src/DecentEthRouter.sol - (uint8 msgType, address _from, address _to, bool deliverEth) = abi - .decode(_payload, (uint8, address, address, bool)); bytes memory callPayload = ""; if (msgType == MT_ETH_TRANSFER_WITH_PAYLOAD) { - (, , , , callPayload) = abi.decode( - _payload, - (uint8, address, address, bool, bytes) - ); + (uint8 msgType, address _from, address _to, bool deliverEth, callPayload) = abi.decode( + _payload, + (uint8, address, address, bool, bytes) + ); + } + else{ + (uint8 msgType, address _from, address _to, bool deliverEth) = abi + .decode(_payload, (uint8, address, address, bool)); + }
#0 - raymondfam
2024-01-26T15:04:13Z
All findings except G-16 are generically complementing the bot report.
#1 - c4-pre-sort
2024-01-26T15:04:18Z
raymondfam marked the issue as sufficient quality report
#2 - alex-ppg
2024-02-04T17:53:01Z
These findings were penalized:
pure
and external
functions. To note, a staticcall
is performed regardless and these functions can become view
in the future, be view
in a subset of adapters, etc.#3 - c4-judge
2024-02-04T17:53:06Z
alex-ppg marked the issue as grade-a