Decent - rjs's results

Decent enables one-click transactions using any token across chains.

General Information

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

Decent

Findings Distribution

Researcher Performance

Rank: 68/113

Findings: 1

Award: $12.28

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

Labels

bug
grade-b
high quality report
QA (Quality Assurance)
sponsor confirmed
Q-10

Awards

12.2818 USDC - $12.28

External Links

Please note: this report has been reviewed to ensure it does not contain automated findings from the bot race winner or 4naly3er

Summary

Low Issues

Non-Critical Issues

Low Issues

[L-01] Best Practice: Follow the Check-Effects-Interaction pattern

Code should follow the best-practice of check-effects-interaction, where state variables are updated before any external calls are made. Doing so prevents a large class of reentrancy bugs.

Instances: 10

File: src/UTB.sol

     |  // @audit-info External call
  76 |  wrapped.deposit{value: swapParams.amountIn}();
     |  // @audit-issue State variable assignment after external call
  77 |  swapParams.tokenIn = address(wrapped);
     |  // @audit-issue State variable assignment after external call
  78 |  swapInstructions.swapPayload = swapper.updateSwapParams(
  79 |      swapParams,
  80 |      swapInstructions.swapPayload
  81 |  );

     |      // @audit-info External call
  83 |      IERC20(swapParams.tokenIn).transferFrom(
  84 |          msg.sender,
  85 |          address(this),
  86 |          swapParams.amountIn
  87 |      );
  :  |
     |  // @audit-info External call
  90 |  IERC20(swapParams.tokenIn).approve(
  91 |      address(swapper),
  92 |      swapParams.amountIn
  93 |  );
  :  |
     |  // @audit-issue State variable assignment after external call
  95 |  (tokenOut, amountOut) = swapper.swap(swapInstructions.swapPayload);

GitHub: 77, 78-81, 95

File: src/UTBExecutor.sol

     |  // @audit-info External call
  61 |  IERC20(token).transferFrom(msg.sender, address(this), amount);
     |  // @audit-info External call
  62 |  IERC20(token).approve(paymentOperator, amount);
  :  |
     |      // @audit-issue State variable assignment after external call
  65 |      (success, ) = target.call{value: extraNative}(payload);
  :  |
     |      // @audit-issue State variable assignment after external call
  70 |      (success, ) = target.call(payload);

GitHub: 65, 70

File: src/swappers/UniSwapper.sol

     |      // @audit-info External call
  83 |      IERC20(swapParams.tokenIn).transferFrom(
  84 |          msg.sender,
  85 |          address(this),
  86 |          swapParams.amountIn
  87 |      );
  :  |
     |  // @audit-issue State variable assignment after external call
  90 |  swapParams.tokenIn = wrapped;

     |  // @audit-info External call
 137 |  IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn);
     |  // @audit-issue State variable assignment after external call
 138 |  amountOut = IV3SwapRouter(uniswap_router).exactInput(params);

     |  // @audit-info External call
 158 |  IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn);
     |  // @audit-issue State variable assignment after external call
 159 |  amountIn = IV3SwapRouter(uniswap_router).exactOutput(params);

GitHub: 90, 138, 159

File: lib/decent-bridge/src/DecentEthRouter.sol

     |  // @audit-info External call
 178 |  weth.deposit{value: _amount}();
     |  // @audit-issue State variable assignment after external call
 179 |  gasValue = msg.value - _amount;

     |  // @audit-info External call
 181 |  weth.transferFrom(msg.sender, address(this), _amount);
     |  // @audit-issue State variable assignment after external call
 182 |  gasValue = msg.value;

GitHub: 179, 182

[L-02] Missing checks for address(0x0) in the constructor/initializer

Since address(0x0) has no private key, it is almost always a mistake when used outside of burning operations.

Instances: 1

File: src/bridge_adapters/DecentBridgeAdapter.sol

     |  // @audit-issue `_bridgeToken` should be checked against 0x0
  21 |  constructor(bool _gasIsEth, address _bridgeToken) BaseAdapter() {

GitHub: 21

[L-03] Missing checks for ecrecover() signature malleability

ecrecover() accepts as valid, two versions of signatures, meaning an attacker can use the same signature twice, or an attacker may be able to front-run the original signer with the altered version of the signature, causing the signer's transaction to revert due to nonce reuse. Consider adding checks for signature malleability, or using OpenZeppelin's ECDSA library to perform the extra checks necessary in order to prevent malleability.

Instances: 1

File: src/UTBFeeCollector.sol

     |  // @audit-issue Verify s or add a nonce
  53 |  address recovered = ecrecover(constructedHash, v, r, s);

GitHub: 53

[L-04] Missing checks for state variable assignments

There are some missing checks in these functions, and this could lead to unexpected scenarios. Consider always adding a sanity check for state variables.

Instances: 11

File: src/UTB.sol

     |  // @audit-issue `swapper` is assigned to a state variable, unvalidated
 325 |  function registerSwapper(address swapper) public onlyOwner {

     |  // @audit-issue `bridge` is assigned to a state variable, unvalidated
 334 |  function registerBridge(address bridge) public onlyOwner {

GitHub: 325, 334

File: src/UTBFeeCollector.sol

     |  // @audit-issue `_signer` is assigned to a state variable, unvalidated
  18 |  function setSigner(address _signer) public onlyOwner {

GitHub: 18

File: src/bridge_adapters/BaseAdapter.sol

     |  // @audit-issue `_executor` is assigned to a state variable, unvalidated
  19 |  function setBridgeExecutor(address _executor) public onlyOwner {

GitHub: 19

File: src/bridge_adapters/DecentBridgeAdapter.sol

     |  // @audit-issue `decentBridgeAdapter` is assigned to a state variable, unvalidated
  34 |  function registerRemoteBridgeAdapter(
  35 |      uint256 dstChainId,
  36 |      uint16 dstLzId,
  37 |      address decentBridgeAdapter
  38 |  ) public onlyOwner {

GitHub: 34-38

File: src/bridge_adapters/StargateBridgeAdapter.sol

     |  // @audit-issue `_sgEth` is assigned to a state variable, unvalidated
  37 |  function setStargateEth(address _sgEth) public onlyOwner {

     |  // @audit-issue `decentBridgeAdapter` is assigned to a state variable, unvalidated
  45 |  function registerRemoteBridgeAdapter(
  46 |      uint256 dstChainId,
  47 |      uint16 dstLzId,
  48 |      address decentBridgeAdapter
  49 |  ) public onlyOwner {

GitHub: 37, 45-49

File: src/swappers/UniSwapper.sol

     |  // @audit-issue `_router` is assigned to a state variable, unvalidated
  20 |  function setRouter(address _router) public onlyOwner {

     |  // @audit-issue `_wrapped` is assigned to a state variable, unvalidated
  24 |  function setWrapped(address payable _wrapped) public onlyOwner {

GitHub: 20, 24

File: lib/decent-bridge/src/DcntEth.sol

     |  // @audit-issue `_router` is assigned to a state variable, unvalidated
  20 |  function setRouter(address _router) public {

GitHub: 20

File: lib/decent-bridge/src/DecentEthRouter.sol

     |  // @audit-issue `_routerAddress` is assigned to a state variable, unvalidated
  73 |  function addDestinationBridge(
  74 |      uint16 _dstChainId,
  75 |      address _routerAddress
  76 |  ) public onlyOwner {

GitHub: 73-76

[L-05] Solidity version 0.8.20 may not work on other chains due to PUSH0

The readme.md specifically states L2s are in scope: Check all that apply (e.g. timelock, NFT, AMM, ERC20, rollups, etc.): ... Uses L2

The compiler for Solidity 0.8.20 switches the default target EVM version to Shanghai, which includes the new PUSH0 op code. This op code may not yet be implemented on all L2s, so deployment on these chains will fail. To work around this issue, use an earlier EVM version. While the project itself may or may not compile with 0.8.20, other projects with which it integrates, or which extend this project may, and those projects will have problems deploying these contracts/libraries.

Instances: 11

File: src/UTB.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.0;

GitHub: 2

File: src/UTBExecutor.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.0;

GitHub: 2

File: src/UTBFeeCollector.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.0;

GitHub: 2

File: src/bridge_adapters/BaseAdapter.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.0;

GitHub: 2

File: src/bridge_adapters/DecentBridgeAdapter.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.0;

GitHub: 2

File: src/bridge_adapters/StargateBridgeAdapter.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.0;

GitHub: 2

File: src/swappers/SwapParams.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.0;

GitHub: 2

File: src/swappers/UniSwapper.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.0;

GitHub: 2

File: lib/decent-bridge/src/DcntEth.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.13;

GitHub: 2

File: lib/decent-bridge/src/DecentEthRouter.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.13;

GitHub: 2

File: lib/decent-bridge/src/DecentBridgeExecutor.sol

     |  // @audit-issue Require Solidity 0.8.19 or lower
   2 |  pragma solidity ^0.8.0;

GitHub: 2

[L-06] Unsafe solidity low-level call can cause gas grief attack

Using the low-level calls of a solidity address can leave the contract open to gas grief attacks. These attacks occur when the called contract returns a large amount of data. So when calling an external contract, it is necessary to check the length of the return data before reading/copying it (using returndatasize()).

Instances: 3

File: src/UTBExecutor.sol

     |  // @audit-issue Check returndatasize()
  52 |  (success, ) = target.call{value: amount}(payload);

     |  // @audit-issue Check returndatasize()
  65 |  (success, ) = target.call{value: extraNative}(payload);

     |  // @audit-issue Check returndatasize()
  70 |  (success, ) = target.call(payload);

GitHub: 52, 65, 70

[L-07] call()/delegatecall() has unchecked return value

The function being called may revert, which will be indicated by the return value to call()/delegatecall(). If the return value is not checked, the code will continue on as if there was no error, rather than reverting with the error encountered.

Instances: 2

File: src/UTBExecutor.sol

     |  // @audit-issue Check the return value of the call
  54 |  (refund.call{value: amount}(""));

     |  // @audit-issue Check the return value of the call
  67 |  (refund.call{value: extraNative}(""));

GitHub: 54, 67

[L-08] call()/delegatecall() should check for contract-existence

Low-level calls return success if there is no code present at the specified address. In addition to the zero-address checks, add a check to verify that <address>.code.length > 0.

Instances: 7

File: src/UTBExecutor.sol

     |  // @audit-issue Add contract existence checks
  52 |  (success, ) = target.call{value: amount}(payload);

     |  // @audit-issue Add contract existence checks
  54 |  (refund.call{value: amount}(""));

     |  // @audit-issue Add contract existence checks
  65 |  (success, ) = target.call{value: extraNative}(payload);

     |  // @audit-issue Add contract existence checks
  67 |  (refund.call{value: extraNative}(""));

     |  // @audit-issue Add contract existence checks
  70 |  (success, ) = target.call(payload);

GitHub: 52, 54, 65, 67, 70

File: lib/decent-bridge/src/DecentBridgeExecutor.sol

     |  // @audit-issue Add contract existence checks
  33 |  (bool success, ) = target.call(callPayload);

     |  // @audit-issue Add contract existence checks
  61 |  (bool success, ) = target.call{value: amount}(callPayload);

GitHub: 33, 61

[L-09] call()/delegatecall() should specify a gas limit

There is no limit specified on the amount of gas used, so the recipient can use up all of the transaction's gas, causing it to revert. Use addr.call{gas: <amount>}("") or this library instead.

Instances: 7

File: src/UTBExecutor.sol

     |  // @audit-issue Specify { gas: <amount> } option
  52 |  (success, ) = target.call{value: amount}(payload);

     |  // @audit-issue Specify { gas: <amount> } option
  54 |  (refund.call{value: amount}(""));

     |  // @audit-issue Specify { gas: <amount> } option
  65 |  (success, ) = target.call{value: extraNative}(payload);

     |  // @audit-issue Specify { gas: <amount> } option
  67 |  (refund.call{value: extraNative}(""));

     |  // @audit-issue Specify { gas: <amount> } option
  70 |  (success, ) = target.call(payload);

GitHub: 52, 54, 65, 67, 70

File: lib/decent-bridge/src/DecentBridgeExecutor.sol

     |  // @audit-issue Specify { gas: <amount> } option
  33 |  (bool success, ) = target.call(callPayload);

     |  // @audit-issue Specify { gas: <amount> } option
  61 |  (bool success, ) = target.call{value: amount}(callPayload);

GitHub: 33, 61

Non-Critical Issues

[N-01] Assembly blocks should have extensive comments

Assembly blocks take a lot more time to audit than normal Solidity code, and often have gotchas and side-effects that the Solidity versions of the same code do not. Consider adding more comments explaining what is being done in every step of the assembly code, and describe why assembly is being used instead of Solidity.

Instances: 1

File: src/UTBFeeCollector.sol

     |  // @audit-issue Inline assembly should be heavily commented
  31 |  assembly {
  32 |      r := mload(add(signature, 32))
  33 |      s := mload(add(signature, 64))
  34 |      v := byte(0, mload(add(signature, 96)))
  35 |  }

GitHub: 31-35

[N-02] Complex arithmetic expression

To maintain readability in code, particularly in Solidity which can involve complex mathematical operations, it is often recommended to limit the number of arithmetic operations to a maximum of 2-3 per line. Too many operations in a single line can make the code difficult to read and understand, increase the likelihood of mistakes, and complicate the process of debugging and reviewing the code. Consider splitting such operations over more than one line, take special care when dealing with division however. Try to limit the number of arithmetic operations to a maximum of 3 per line.

Instances: 2

File: src/bridge_adapters/StargateBridgeAdapter.sol

     |  // @audit-issue Simplify by using intermediate variables
  66 |  return (amt2Bridge * (1e4 - SG_FEE_BPS)) / 1e4;

     |  // @audit-issue Simplify by using intermediate variables
 170 |  router.swap{value: msg.value}(
 171 |      getDstChainId(additionalArgs), //lzBridgeData._dstChainId, // send to LayerZero chainId
 172 |      getSrcPoolId(additionalArgs), //lzBridgeData._srcPoolId, // source pool id
 173 |      getDstPoolId(additionalArgs), //lzBridgeData._dstPoolId, // dst pool id
 174 |      refund, // refund adddress. extra gas (if any) is returned to this address
 175 |      amt2Bridge, // quantity to swap
 176 |      (amt2Bridge * (10000 - SG_FEE_BPS)) / 10000, // the min qty you would accept on the destination, fee is 6 bips
 177 |      getLzTxObj(additionalArgs), // additional gasLimit increase, airdrop, at address
 178 |      abi.encodePacked(getDestAdapter(dstChainId)),
 179 |      bridgePayload // bytes param, if you wish to send additional payload you can abi.encode() them here
 180 |  );

GitHub: 66, 170-180

[N-03] Consider adding a block/deny-list

Doing so will significantly increase centralization, but will help to prevent hackers from using stolen tokens.

Instances: 1

File: lib/decent-bridge/src/DecentEthRouter.sol

     |  // @audit-issue Consider adding a block/deny-list
  12 |  contract DecentEthRouter is IDecentEthRouter, IOFTReceiverV2, Owned {

GitHub: 12

[N-04] Consider moving msg.sender checks to modifiers

Using modifiers makes the contract guard intention of the code more clear, and also guarantees no other code can run before the modifier runs.

Instances: 3

File: src/bridge_adapters/BaseAdapter.sol

     |  // @audit-issue Refactor into a modifier
  12 |  require(
  13 |      msg.sender == address(bridgeExecutor),
  14 |      "Only bridge executor can call this"
  15 |  );

GitHub: 12-15

File: lib/decent-bridge/src/DcntEth.sol

     |  // @audit-issue Refactor into a modifier
   9 |  require(msg.sender == router);

GitHub: 9

File: lib/decent-bridge/src/DecentEthRouter.sol

     |  // @audit-issue Refactor into a modifier
  43 |  require(
  44 |      address(dcntEth) == msg.sender,
  45 |      "DecentEthRouter: only lz App can call"
  46 |  );

GitHub: 43-46

[N-05] Consider using descriptive constants when passing zero as a function argument

Passing 0 or 0x0 as a function argument can sometimes result in a security issue(e.g. passing zero as the slippage parameter). A historical example is the infamous 0x0 address bug where numerous tokens were lost. This happens because 0 can be interpreted as an uninitialized address, leading to transfers to the 0x0 address, effectively burning tokens. Moreover, 0 as a denominator in division operations would cause a runtime exception. It's also often indicative of a logical error in the caller's code.

Consider using a constant variable with a descriptive name, so it's clear that the argument is intentionally being used, and for the right reasons.

Instances: 1

File: src/UTBExecutor.sol

     |  // @audit-issue Replace literal 0 with an explicit constant
  28 |  execute(target, paymentOperator, payload, token, amount, refund, 0);

GitHub: 28

[N-06] Consider using modifiers for access control

Modifiers in Solidity can improve code readability and modularity by encapsulating repetitive checks, such as address validity checks, into a reusable construct. For example, an onlyOwner modifier can be used to replace repetitive require(msg.sender == owner) checks across several functions, reducing code redundancy and enhancing maintainability. To implement, define a modifier with the check, then apply the modifier to relevant functions.

Instances: 3

File: src/bridge_adapters/BaseAdapter.sol

     |  // @audit-issue Refactor into a modifier
  12 |  require(
  13 |      msg.sender == address(bridgeExecutor),
  14 |      "Only bridge executor can call this"
  15 |  );

GitHub: 12-15

File: lib/decent-bridge/src/DcntEth.sol

     |  // @audit-issue Refactor into a modifier
   9 |  require(msg.sender == router);

GitHub: 9

File: lib/decent-bridge/src/DecentEthRouter.sol

     |  // @audit-issue Refactor into a modifier
  43 |  require(
  44 |      address(dcntEth) == msg.sender,
  45 |      "DecentEthRouter: only lz App can call"
  46 |  );

GitHub: 43-46

[N-07] Consider using named function call parameters when >= 4 parameters

Named function calls in Solidity greatly improve code readability by explicitly mapping arguments to their respective parameter names. This clarity becomes critical when dealing with functions that have numerous or complex parameters, reducing potential errors due to misordered arguments. Therefore, adopting named function calls contributes to more maintainable and less error-prone code. The following findings are for function calls with 4 or more praameters.

Instances: 28

File: src/UTB.sol

     |  // @audit-issue Consider named parameters
 117 |  _swapAndExecute(
 118 |      instructions.swapInstructions,
 119 |      instructions.target,
 120 |      instructions.paymentOperator,
 121 |      instructions.payload,
 122 |      instructions.refund
 123 |  );

     |  // @audit-issue Consider named parameters
 143 |  executor.execute{value: amountOut}(
 144 |      target,
 145 |      paymentOperator,
 146 |      payload,
 147 |      tokenOut,
 148 |      amountOut,
 149 |      refund
 150 |  );

     |  // @audit-issue Consider named parameters
 153 |  executor.execute(
 154 |      target,
 155 |      paymentOperator,
 156 |      payload,
 157 |      tokenOut,
 158 |      amountOut,
 159 |      refund
 160 |  );

     |  // @audit-issue Consider named parameters
 289 |  IBridgeAdapter(bridgeAdapters[instructions.bridgeId]).bridge{
 290 |      value: bridgeFee + (native ? amt2Bridge : 0)
 291 |  }(
 292 |      amt2Bridge,
 293 |      instructions.postBridge,
 294 |      instructions.dstChainId,
 295 |      instructions.target,
 296 |      instructions.paymentOperator,
 297 |      instructions.payload,
 298 |      instructions.additionalArgs,
 299 |      instructions.refund
 300 |  );

     |  // @audit-issue Consider named parameters
 318 |  _swapAndExecute(postBridge, target, paymentOperator, payload, refund);

GitHub: 117-123, 143-150, 153-160, 289-300, 318

File: src/UTBExecutor.sol

     |  // @audit-issue Consider named parameters
  28 |  execute(target, paymentOperator, payload, token, amount, refund, 0);

GitHub: 28

File: src/UTBFeeCollector.sol

     |  // @audit-issue Consider named parameters
  53 |  address recovered = ecrecover(constructedHash, v, r, s);

GitHub: 53

File: src/bridge_adapters/DecentBridgeAdapter.sol

     |  // @audit-issue Consider named parameters
  56 |  router.estimateSendAndCallFee(
  57 |      router.MT_ETH_TRANSFER_WITH_PAYLOAD(),
  58 |      lzIdLookup[dstChainId],
  59 |      target,
  60 |      swapParams.amountIn,
  61 |      dstGas,
  62 |      false,
  63 |      payload
  64 |  );

     |  // @audit-issue Consider named parameters
 117 |  router.bridgeWithPayload{value: msg.value}(
 118 |      lzIdLookup[dstChainId],
 119 |      destinationBridgeAdapter[dstChainId],
 120 |      swapParams.amountIn,
 121 |      false,
 122 |      dstGas,
 123 |      bridgePayload
 124 |  );

     |  // @audit-issue Consider named parameters
 147 |  IUTB(utb).receiveFromBridge(
 148 |      postBridge,
 149 |      target,
 150 |      paymentOperator,
 151 |      payload,
 152 |      refund
 153 |  );

GitHub: 56-64, 117-124, 147-153

File: src/bridge_adapters/StargateBridgeAdapter.sol

     |  // @audit-issue Consider named parameters
  81 |  bridgePayload = abi.encode(
  82 |      postBridge,
  83 |      target,
  84 |      paymentOperator,
  85 |      payload,
  86 |      refund
  87 |  );

     |  // @audit-issue Consider named parameters
  91 |  callBridge(
  92 |      amt2Bridge,
  93 |      dstChainId,
  94 |      bridgePayload,
  95 |      additionalArgs,
  96 |      refund
  97 |  );

     |  // @audit-issue Consider named parameters
 170 |  router.swap{value: msg.value}(
 171 |      getDstChainId(additionalArgs), //lzBridgeData._dstChainId, // send to LayerZero chainId
 172 |      getSrcPoolId(additionalArgs), //lzBridgeData._srcPoolId, // source pool id
 173 |      getDstPoolId(additionalArgs), //lzBridgeData._dstPoolId, // dst pool id
 174 |      refund, // refund adddress. extra gas (if any) is returned to this address
 175 |      amt2Bridge, // quantity to swap
 176 |      (amt2Bridge * (10000 - SG_FEE_BPS)) / 10000, // the min qty you would accept on the destination, fee is 6 bips
 177 |      getLzTxObj(additionalArgs), // additional gasLimit increase, airdrop, at address
 178 |      abi.encodePacked(getDestAdapter(dstChainId)),
 179 |      bridgePayload // bytes param, if you wish to send additional payload you can abi.encode() them here
 180 |  );

     |  // @audit-issue Consider named parameters
 209 |  IUTB(utb).receiveFromBridge(
 210 |      postBridge,
 211 |      target,
 212 |      paymentOperator,
 213 |      utbPayload,
 214 |      refund
 215 |  );

GitHub: 81-87, 91-97, 170-180, 209-215

File: src/swappers/UniSwapper.sol

     |  // @audit-issue Consider named parameters
 129 |  IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter
 130 |      .ExactInputParams({
 131 |          path: swapParams.path,
 132 |          recipient: address(this),
 133 |          amountIn: swapParams.amountIn,
 134 |          amountOutMinimum: swapParams.amountOut
 135 |      });

     |  // @audit-issue Consider named parameters
 149 |  IV3SwapRouter.ExactOutputParams memory params = IV3SwapRouter
 150 |      .ExactOutputParams({
 151 |          path: swapParams.path,
 152 |          recipient: address(this),
 153 |          //deadline: block.timestamp,
 154 |          amountOut: swapParams.amountOut,
 155 |          amountInMaximum: swapParams.amountIn
 156 |      });

GitHub: 129-135, 149-156

File: lib/decent-bridge/src/DecentEthRouter.sol

     |  // @audit-issue Consider named parameters
 101 |  payload = abi.encode(msgType, msg.sender, _toAddress, deliverEth);

     |  // @audit-issue Consider named parameters
 103 |  payload = abi.encode(
 104 |      msgType,
 105 |      msg.sender,
 106 |      _toAddress,
 107 |      deliverEth,
 108 |      additionalPayload
 109 |  );

     |  // @audit-issue Consider named parameters
 126 |  ) = _getCallParams(
 127 |          msgType,
 128 |          _toAddress,
 129 |          _dstChainId,
 130 |          _dstGasForCall,
 131 |          deliverEth,
 132 |          payload
 133 |      );

     |  // @audit-issue Consider named parameters
 136 |  dcntEth.estimateSendAndCallFee(
 137 |      _dstChainId,
 138 |      destinationBridge,
 139 |      _amount,
 140 |      _payload,
 141 |      _dstGasForCall,
 142 |      false, // useZero
 143 |      adapterParams // Relayer adapter parameters
 144 |      // contains packet type (send and call in this case) and gasAmount
 145 |  );

     |  // @audit-issue Consider named parameters
 161 |  ) = _getCallParams(
 162 |          msgType,
 163 |          _toAddress,
 164 |          _dstChainId,
 165 |          _dstGasForCall,
 166 |          deliverEth,
 167 |          additionalPayload
 168 |      );

     |  // @audit-issue Consider named parameters
 185 |  dcntEth.sendAndCall{value: gasValue}(
 186 |      address(this), // from address that has dcntEth (so DecentRouter)
 187 |      _dstChainId,
 188 |      destinationBridge, // toAddress
 189 |      _amount, // amount
 190 |      payload, //payload (will have recipients address)
 191 |      _dstGasForCall, // dstGasForCall
 192 |      callParams // refundAddress, zroPaymentAddress, adapterParams
 193 |  );

     |  // @audit-issue Consider named parameters
 206 |  _bridgeWithPayload(
 207 |      MT_ETH_TRANSFER_WITH_PAYLOAD,
 208 |      _dstChainId,
 209 |      _toAddress,
 210 |      _amount,
 211 |      _dstGasForCall,
 212 |      additionalPayload,
 213 |      deliverEth
 214 |  );

     |  // @audit-issue Consider named parameters
 225 |  _bridgeWithPayload(
 226 |      MT_ETH_TRANSFER,
 227 |      _dstChainId,
 228 |      _toAddress,
 229 |      _amount,
 230 |      _dstGasForCall,
 231 |      bytes(""),
 232 |      deliverEth
 233 |  );

     |  // @audit-issue Consider named parameters
 257 |  emit ReceivedDecentEth(
 258 |      msgType,
 259 |      _srcChainId,
 260 |      _from,
 261 |      _to,
 262 |      _amount,
 263 |      callPayload
 264 |  );

     |  // @audit-issue Consider named parameters
 280 |  executor.execute(_from, _to, deliverEth, _amount, callPayload);

GitHub: 101, 103-109, 126-133, 136-145, 161-168, 185-193, 206-214, 225-233, 257-264, 280

File: lib/decent-bridge/src/DecentBridgeExecutor.sol

     |  // @audit-issue Consider named parameters
  78 |  _executeWeth(from, target, amount, callPayload);

     |  // @audit-issue Consider named parameters
  80 |  _executeEth(from, target, amount, callPayload);

GitHub: 78, 80

[N-08] Duplicate strings defined

For better maintainability, consider creating and using a constant for those strings instead of duplicating it.

Instances: 1

File: src/bridge_adapters/DecentBridgeAdapter.sol

     |  // @audit-info Duplicates of this string were detected
  93 |  string.concat("dst chain address not set ")

File: src/bridge_adapters/StargateBridgeAdapter.sol
     |  // @audit-issue First seen at src/bridge_adapters/DecentBridgeAdapter.sol:93
 159 |  string.concat("dst chain address not set ")

GitHub: 159

[N-09] Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, for readability

Well-organized data structures make code reviews easier, which may lead to fewer bugs. Consider combining related mappings into mappings to structs, so it's clear what data is related.

Instances: 2

File: src/bridge_adapters/StargateBridgeAdapter.sol

     |  // @audit-issue Consider whether multiple mappings can be merged into a single struct mapping
  25 |  mapping(uint256 => address) public destinationBridgeAdapter;

     |  // @audit-issue Consider whether multiple mappings can be merged into a single struct mapping
  26 |  mapping(uint256 => uint16) lzIdLookup;

GitHub: 25, 26

[N-10] NatSpec: public state variable declarations should have @notice tag

As per Solidity NatSpec.

Instances: 25

File: src/UTB.sol

     |  // @audit-issue Add NatSpec documentation
  21 |  mapping(uint8 => address) public swappers;

     |  // @audit-issue Add NatSpec documentation
  22 |  mapping(uint8 => address) public bridgeAdapters;

GitHub: 21, 22

File: src/bridge_adapters/BaseAdapter.sol

     |  // @audit-issue Add NatSpec documentation
   7 |  address public bridgeExecutor;

GitHub: 7

File: src/bridge_adapters/DecentBridgeAdapter.sol

     |  // @audit-issue Add NatSpec documentation
  13 |  uint8 public constant BRIDGE_ID = 0;

     |  // @audit-issue Add NatSpec documentation
  14 |  mapping(uint256 => address) public destinationBridgeAdapter;

     |  // @audit-issue Add NatSpec documentation
  15 |  IDecentEthRouter public router;

GitHub: 13, 14, 15

File: src/bridge_adapters/StargateBridgeAdapter.sol

     |  // @audit-issue Add NatSpec documentation
  22 |  uint8 public constant BRIDGE_ID = 1;

     |  // @audit-issue Add NatSpec documentation
  23 |  uint8 public constant SG_FEE_BPS = 6;

     |  // @audit-issue Add NatSpec documentation
  24 |  address public stargateEth;

     |  // @audit-issue Add NatSpec documentation
  25 |  mapping(uint256 => address) public destinationBridgeAdapter;

     |  // @audit-issue Add NatSpec documentation
  31 |  IStargateRouter public router;

GitHub: 22, 23, 24, 25, 31

File: src/swappers/UniSwapper.sol

     |  // @audit-issue Add NatSpec documentation
  16 |  uint8 public constant SWAPPER_ID = 0;

     |  // @audit-issue Add NatSpec documentation
  17 |  address public uniswap_router;

     |  // @audit-issue Add NatSpec documentation
  18 |  address payable public wrapped;

GitHub: 16, 17, 18

File: lib/decent-bridge/src/DcntEth.sol

     |  // @audit-issue Add NatSpec documentation
   6 |  address public router;

GitHub: 6

File: lib/decent-bridge/src/DecentEthRouter.sol

     |  // @audit-issue Add NatSpec documentation
  13 |  IWETH public weth;

     |  // @audit-issue Add NatSpec documentation
  14 |  IDcntEth public dcntEth;

     |  // @audit-issue Add NatSpec documentation
  15 |  IDecentBridgeExecutor public executor;

     |  // @audit-issue Add NatSpec documentation
  17 |  uint8 public constant MT_ETH_TRANSFER = 0;

     |  // @audit-issue Add NatSpec documentation
  18 |  uint8 public constant MT_ETH_TRANSFER_WITH_PAYLOAD = 1;

     |  // @audit-issue Add NatSpec documentation
  20 |  uint16 public constant PT_SEND_AND_CALL = 1;

     |  // @audit-issue Add NatSpec documentation
  22 |  bool public gasCurrencyIsEth; // for chains that use ETH as gas currency

     |  // @audit-issue Add NatSpec documentation
  24 |  mapping(uint16 => address) public destinationBridges;

     |  // @audit-issue Add NatSpec documentation
  25 |  mapping(address => uint256) public balanceOf;

GitHub: 13, 14, 15, 17, 18, 20, 22, 24, 25

File: lib/decent-bridge/src/DecentBridgeExecutor.sol

     |  // @audit-issue Add NatSpec documentation
  10 |  bool public gasCurrencyIsEth; // for chains that use ETH as gas currency

GitHub: 10

[N-11] NatSpec: state variable declarations should have @dev tag

As per Solidity NatSpec. Note: public and non-public state variables should have a @dev tag. Only public state variables should have a @notice tag.

Instances: 39

File: src/UTB.sol

     |  // @audit-issue Add NatSpec documentation
  18 |  IUTBExecutor executor;

     |  // @audit-issue Add NatSpec documentation
  19 |  IUTBFeeCollector feeCollector;

     |  // @audit-issue Add NatSpec documentation
  20 |  IWETH wrapped;

     |  // @audit-issue Add NatSpec documentation
  21 |  mapping(uint8 => address) public swappers;

     |  // @audit-issue Add NatSpec documentation
  22 |  mapping(uint8 => address) public bridgeAdapters;

GitHub: 18, 19, 20, 21, 22

File: src/UTBFeeCollector.sol

     |  // @audit-issue Add NatSpec documentation
   9 |  address signer;

     |  // @audit-issue Add NatSpec documentation
  10 |  string constant BANNER = "\x19Ethereum Signed Message:\n32";

GitHub: 9, 10

File: src/bridge_adapters/BaseAdapter.sol

     |  // @audit-issue Add NatSpec documentation
   7 |  address public bridgeExecutor;

GitHub: 7

File: src/bridge_adapters/DecentBridgeAdapter.sol

     |  // @audit-issue Add NatSpec documentation
  13 |  uint8 public constant BRIDGE_ID = 0;

     |  // @audit-issue Add NatSpec documentation
  14 |  mapping(uint256 => address) public destinationBridgeAdapter;

     |  // @audit-issue Add NatSpec documentation
  15 |  IDecentEthRouter public router;

     |  // @audit-issue Add NatSpec documentation
  16 |  mapping(uint256 => uint16) lzIdLookup;

     |  // @audit-issue Add NatSpec documentation
  17 |  mapping(uint16 => uint256) chainIdLookup;

     |  // @audit-issue Add NatSpec documentation
  18 |  bool gasIsEth;

     |  // @audit-issue Add NatSpec documentation
  19 |  address bridgeToken;

GitHub: 13, 14, 15, 16, 17, 18, 19

File: src/bridge_adapters/StargateBridgeAdapter.sol

     |  // @audit-issue Add NatSpec documentation
  22 |  uint8 public constant BRIDGE_ID = 1;

     |  // @audit-issue Add NatSpec documentation
  23 |  uint8 public constant SG_FEE_BPS = 6;

     |  // @audit-issue Add NatSpec documentation
  24 |  address public stargateEth;

     |  // @audit-issue Add NatSpec documentation
  25 |  mapping(uint256 => address) public destinationBridgeAdapter;

     |  // @audit-issue Add NatSpec documentation
  26 |  mapping(uint256 => uint16) lzIdLookup;

     |  // @audit-issue Add NatSpec documentation
  27 |  mapping(uint16 => uint256) chainIdLookup;

     |  // @audit-issue Add NatSpec documentation
  31 |  IStargateRouter public router;

GitHub: 22, 23, 24, 25, 26, 27, 31

File: src/swappers/SwapParams.sol

     |  // @audit-issue Add NatSpec documentation
   5 |  uint8 constant EXACT_IN = 0;

     |  // @audit-issue Add NatSpec documentation
   6 |  uint8 constant EXACT_OUT = 1;

GitHub: 5, 6

File: src/swappers/UniSwapper.sol

     |  // @audit-issue Add NatSpec documentation
  16 |  uint8 public constant SWAPPER_ID = 0;

     |  // @audit-issue Add NatSpec documentation
  17 |  address public uniswap_router;

     |  // @audit-issue Add NatSpec documentation
  18 |  address payable public wrapped;

GitHub: 16, 17, 18

File: lib/decent-bridge/src/DcntEth.sol

     |  // @audit-issue Add NatSpec documentation
   6 |  address public router;

GitHub: 6

File: lib/decent-bridge/src/DecentEthRouter.sol

     |  // @audit-issue Add NatSpec documentation
  13 |  IWETH public weth;

     |  // @audit-issue Add NatSpec documentation
  14 |  IDcntEth public dcntEth;

     |  // @audit-issue Add NatSpec documentation
  15 |  IDecentBridgeExecutor public executor;

     |  // @audit-issue Add NatSpec documentation
  17 |  uint8 public constant MT_ETH_TRANSFER = 0;

     |  // @audit-issue Add NatSpec documentation
  18 |  uint8 public constant MT_ETH_TRANSFER_WITH_PAYLOAD = 1;

     |  // @audit-issue Add NatSpec documentation
  20 |  uint16 public constant PT_SEND_AND_CALL = 1;

     |  // @audit-issue Add NatSpec documentation
  22 |  bool public gasCurrencyIsEth; // for chains that use ETH as gas currency

     |  // @audit-issue Add NatSpec documentation
  24 |  mapping(uint16 => address) public destinationBridges;

     |  // @audit-issue Add NatSpec documentation
  25 |  mapping(address => uint256) public balanceOf;

GitHub: 13, 14, 15, 17, 18, 20, 22, 24, 25

File: lib/decent-bridge/src/DecentBridgeExecutor.sol

     |  // @audit-issue Add NatSpec documentation
   9 |  IWETH weth;

     |  // @audit-issue Add NatSpec documentation
  10 |  bool public gasCurrencyIsEth; // for chains that use ETH as gas currency

GitHub: 9, 10

[N-12] Redundant else block

One level of nesting can be removed by not having an else block when the if-block returns.

Instances: 2

File: src/UTBExecutor.sol

     |  // @audit-issue The else clause is unnecessary
  76 |  if (remainingBalance == 0) {
  77 |      return;
  78 |  }

GitHub: 76-78

File: src/swappers/UniSwapper.sol

     |  // @audit-issue The else clause is unnecessary
  68 |  if (swapParams.path.length == 0) {
  69 |      return swapNoPath(swapParams, receiver, refund);
  70 |  }

GitHub: 68-70

[N-13] State variables should include comments

Consider adding some comments on critical state variables to explain what they are supposed to do: this will help for future code reviews.

Instances: 39

File: src/UTB.sol

     |  // @audit-issue Consider adding comments explaining this state var
  18 |  IUTBExecutor executor;

     |  // @audit-issue Consider adding comments explaining this state var
  19 |  IUTBFeeCollector feeCollector;

     |  // @audit-issue Consider adding comments explaining this state var
  20 |  IWETH wrapped;

     |  // @audit-issue Consider adding comments explaining this state var
  21 |  mapping(uint8 => address) public swappers;

     |  // @audit-issue Consider adding comments explaining this state var
  22 |  mapping(uint8 => address) public bridgeAdapters;

GitHub: 18, 19, 20, 21, 22

File: src/UTBFeeCollector.sol

     |  // @audit-issue Consider adding comments explaining this state var
   9 |  address signer;

     |  // @audit-issue Consider adding comments explaining this state var
  10 |  string constant BANNER = "\x19Ethereum Signed Message:\n32";

GitHub: 9, 10

File: src/bridge_adapters/BaseAdapter.sol

     |  // @audit-issue Consider adding comments explaining this state var
   7 |  address public bridgeExecutor;

GitHub: 7

File: src/bridge_adapters/DecentBridgeAdapter.sol

     |  // @audit-issue Consider adding comments explaining this state var
  13 |  uint8 public constant BRIDGE_ID = 0;

     |  // @audit-issue Consider adding comments explaining this state var
  14 |  mapping(uint256 => address) public destinationBridgeAdapter;

     |  // @audit-issue Consider adding comments explaining this state var
  15 |  IDecentEthRouter public router;

     |  // @audit-issue Consider adding comments explaining this state var
  16 |  mapping(uint256 => uint16) lzIdLookup;

     |  // @audit-issue Consider adding comments explaining this state var
  17 |  mapping(uint16 => uint256) chainIdLookup;

     |  // @audit-issue Consider adding comments explaining this state var
  18 |  bool gasIsEth;

     |  // @audit-issue Consider adding comments explaining this state var
  19 |  address bridgeToken;

GitHub: 13, 14, 15, 16, 17, 18, 19

File: src/bridge_adapters/StargateBridgeAdapter.sol

     |  // @audit-issue Consider adding comments explaining this state var
  22 |  uint8 public constant BRIDGE_ID = 1;

     |  // @audit-issue Consider adding comments explaining this state var
  23 |  uint8 public constant SG_FEE_BPS = 6;

     |  // @audit-issue Consider adding comments explaining this state var
  24 |  address public stargateEth;

     |  // @audit-issue Consider adding comments explaining this state var
  25 |  mapping(uint256 => address) public destinationBridgeAdapter;

     |  // @audit-issue Consider adding comments explaining this state var
  26 |  mapping(uint256 => uint16) lzIdLookup;

     |  // @audit-issue Consider adding comments explaining this state var
  27 |  mapping(uint16 => uint256) chainIdLookup;

     |  // @audit-issue Consider adding comments explaining this state var
  31 |  IStargateRouter public router;

GitHub: 22, 23, 24, 25, 26, 27, 31

File: src/swappers/SwapParams.sol

     |  // @audit-issue Consider adding comments explaining this state var
   5 |  uint8 constant EXACT_IN = 0;

     |  // @audit-issue Consider adding comments explaining this state var
   6 |  uint8 constant EXACT_OUT = 1;

GitHub: 5, 6

File: src/swappers/UniSwapper.sol

     |  // @audit-issue Consider adding comments explaining this state var
  16 |  uint8 public constant SWAPPER_ID = 0;

     |  // @audit-issue Consider adding comments explaining this state var
  17 |  address public uniswap_router;

     |  // @audit-issue Consider adding comments explaining this state var
  18 |  address payable public wrapped;

GitHub: 16, 17, 18

File: lib/decent-bridge/src/DcntEth.sol

     |  // @audit-issue Consider adding comments explaining this state var
   6 |  address public router;

GitHub: 6

File: lib/decent-bridge/src/DecentEthRouter.sol

     |  // @audit-issue Consider adding comments explaining this state var
  13 |  IWETH public weth;

     |  // @audit-issue Consider adding comments explaining this state var
  14 |  IDcntEth public dcntEth;

     |  // @audit-issue Consider adding comments explaining this state var
  15 |  IDecentBridgeExecutor public executor;

     |  // @audit-issue Consider adding comments explaining this state var
  17 |  uint8 public constant MT_ETH_TRANSFER = 0;

     |  // @audit-issue Consider adding comments explaining this state var
  18 |  uint8 public constant MT_ETH_TRANSFER_WITH_PAYLOAD = 1;

     |  // @audit-issue Consider adding comments explaining this state var
  20 |  uint16 public constant PT_SEND_AND_CALL = 1;

     |  // @audit-issue Consider adding comments explaining this state var
  22 |  bool public gasCurrencyIsEth; // for chains that use ETH as gas currency

     |  // @audit-issue Consider adding comments explaining this state var
  24 |  mapping(uint16 => address) public destinationBridges;

     |  // @audit-issue Consider adding comments explaining this state var
  25 |  mapping(address => uint256) public balanceOf;

GitHub: 13, 14, 15, 17, 18, 20, 22, 24, 25

File: lib/decent-bridge/src/DecentBridgeExecutor.sol

     |  // @audit-issue Consider adding comments explaining this state var
   9 |  IWETH weth;

     |  // @audit-issue Consider adding comments explaining this state var
  10 |  bool public gasCurrencyIsEth; // for chains that use ETH as gas currency

GitHub: 9, 10

[N-14] Syntax: variables need not be initialized to zero

The default value for variables is zero, so initializing them to zero is superfluous. This will also save gas if the variable is a mutable state variable.

Instances: 1

File: src/UTB.sol

     |  // @audit-issue Initial value is unnecessary
 234 |  uint value = 0;

GitHub: 234

[N-15] Typos

Consider correcting these spelling errors.

Instances: 1

File: src/bridge_adapters/StargateBridgeAdapter.sol

     |  // @audit-issue Typo: adddress
 174 |  refund, // refund adddress. extra gas (if any) is returned to this address

GitHub: 174

[N-16] Unnecessary cast

Unnecessary casts can be removed.

Instances: 1

File: src/bridge_adapters/BaseAdapter.sol

     |  // @audit-issue Cast to address is redundant
  13 |  msg.sender == address(bridgeExecutor),

GitHub: 13

[N-17] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead. If all arguments are strings and or bytes, bytes.concat() should be used instead

Instances: 1

File: src/UTBFeeCollector.sol

     |  // @audit-issue Use abi.encode instead to prevent dynamic-length hash collisions
  49 |  bytes32 constructedHash = keccak256(
  50 |      abi.encodePacked(BANNER, keccak256(packedInfo))
  51 |  );

GitHub: 49-51

[N-18] require()/revert() statements should have descriptive reason strings

The error message should clearly state how/why something is an error, not just that a specific error state was reached; someone shouldn't have to read the code in order to see how to resolve the problem.

Instances: 1

File: lib/decent-bridge/src/DcntEth.sol

     |  // @audit-issue Add an error description, ideally with a custom error
   9 |  require(msg.sender == router);

GitHub: 9

#0 - raymondfam

2024-01-26T07:54:56Z

[L-01], [L-02], [L-4]: Generically known and reported by the bot(s) [L-05]: Hardhat has Paris resolved [L-06] - [L-9]: Generically known and reported by the bot(s) {N-01] - [N-18]: Generically known and reported by the bot(s)

Overall, a good complement to the selected bot.

#1 - c4-pre-sort

2024-01-26T07:55:18Z

raymondfam marked the issue as high quality report

#2 - c4-sponsor

2024-01-30T16:53:45Z

wkantaros (sponsor) confirmed

#3 - alex-ppg

2024-02-04T22:48:32Z

QA Judgment

The Warden's QA report has been graded B based on a score of 26 combined with a manual review per the relevant QA guideline document located here.

The Warden's submission's score was assessed based on the following accepted findings:

Low-Risk

  • L-06
  • L-08

Non-Critical

  • L-03
  • L-05: #532

#4 - c4-judge

2024-02-04T22:48:35Z

alex-ppg marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter