The DcntEth contract is designed to interface with the DecentEthRouter contract, which is intended to be the sole entity capable of calling the functions mint() and burn(). These functions are critical as they directly affect the token supply of DcntEth. The setRouter(address _router) function is meant to establish the router's address, which is then enforced by the onlyRouter modifier on sensitive functions.
However, the current implementation of setRouter() lacks any form of access control, allowing any user to set themselves as the router. This poses a severe security risk as an unauthorized user could call setRouter() to set their address as the router and subsequently invoke mint() to arbitrarily increase the token supply in their favor, leading to potential token inflation and devaluation.
- An attacker deploys their own contract or uses an EOA.
- The attacker calls
setRouter()with their address. - The attacker is now set as the router and can call
mint()to mint an arbitrary amount ofDcntEthto any address they choose.
Manual review
To mitigate this issue, implement access control in the setRouter() function. This can be achieved by adding the onlyOwner modifier.
The code fix would be as follows:
function setRouter(address _router) public onlyOwner {
router = _router;
}This change ensures that only the owner of the contract can set or change the router, preventing unauthorized manipulation of the token supply.
DecentEthRouter.sol#L197
DecentEthRouter.sol#L103-L109
DecentEthRouter.sol#L161
This issue is internally the same as described in code-423n4/2023-07-tapioca-findings#1220. To avoid duplication of efforts and unnecessary copy-pasting, only the differences are explained in this issue.
The entry point here is the bridgeWithPayload() function in the DecentEthRouter contract, which allows the caller to pass a variable-length additionalPayload variable. This variable is then encoded as part of the payload variable returned by _getCallParams() and passed as parameter to DcntEth.sendAndCall(). This is a function that DcntEth inherits from OFTV2, which inherits it from BaseOFTV2 . This function calls the internal _sendAndCall() function inherited from OFTCoreV2, which finally calls _lzSend(). This ties into the description of the vulnerability in the aforementioned issue.
On the other end, the message will be processed in the onOFTReceived() function of the same contract, which is invoked in the OFTCoreV2.callOnOFTReceived() function when receiving a message. Our payload will be extracted as the callPayload variable and passed on to the executor, where the msgType is hardcoded as MT_ETH_TRANSFER_WITH_PAYLOAD in bridgeWithPayload(). Finally, in DecentBridgeExecutor, we always get a call to our target address with the payload we defined (of which only its size matters), in which we can drain as much gas as needed for the attack. This completes the exploit as all intermediary steps are explained in great detail in the issue referenced above.
Manual review
The recommended mitigation is similar to the one in the reference issue. Alternatively or in addition to it, a maximum size for any variable-length parameters that can be passed in a LayerZero message to an arbitrary address could be implemented.
[H-03] Failed calls made via bridgeWithPayload() may transfer funds to address not under user control
DecentBridgeExecutor.sol#L36
DecentBridgeExecutor.sol#L63
The DecentBridgeExecutor contract has two private functions, _executeWeth() and _executeEth(), which are used to execute transactions with a custom payload received through the Decent bridge.
In both functions, if the call to the target contract fails, the contract sends the funds back to the from address. This behavior is problematic because the from address is hardcoded as msg.sender in the _getCallParams() function on the sender side:
https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L100-L109
if (msgType == MT_ETH_TRANSFER) {
payload = abi.encode(msgType, msg.sender, _toAddress, deliverEth);
} else {
payload = abi.encode(
msgType,
msg.sender,
_toAddress,
deliverEth,
additionalPayload
);The problem is that the sender in the source chain may not be in control of the same address in the destination chain. For instance, this could occur if the sender is a Gnosis Safe created using the createProxy() function of the Proxy Factory. This issue has been exploited in the past to steal 20 million $OP.
- Alice owns a Gnosis Safe deployed using the
createProxy()function of the Gnosis Safe Proxy Factory. - Alice initiates a cross-chain transaction from her safe through the
DecentEthRouter.bridgeWithPayload()function. The_getCallParams()function hardcodes the address of Alice's safe on the source chain as thefromaddress. - The transaction is processed by either the
_executeWeth()or_executeEth()function in theDecentBridgeExecutorcontract of the target chain. - The transaction call to the target contract fails due to an issue outside of Alice's control, such as the target contract being paused.
- The
DecentBridgeExecutorcontract sends the funds to thefromaddress, which is the address of Alice's safe on the source chain. - However, since Alice does not control this address on the destination chain, she is unable to access these funds. This results in a loss of funds for Alice.
Manual review
To mitigate this issue, it is recommended to revert the transaction if the call to the target contract fails, instead of sending the funds to the from address. Since the DcntEth contract is a NonblockingLzApp, this is the recommended behavior as failed transactions are stored and can be retried after they fail.
The DecentEthRouter.onOFTReceived() function is responsible for handling incoming messages from the Decent bridge. It decodes the payload of the transfer to extract necessary information such as the type of message, sender, recipient, and whether the transfer should be in ETH or WETH.
If the message type is MT_ETH_TRANSFER_WITH_PAYLOAD and the sender sets deliverEth to false, the transaction is expected to allow the target contract to pull the WETH amount in a call with the given payload:
https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentBridgeExecutor.sol#L31-L33
weth.approve(target, amount);
(bool success, ) = target.call(callPayload);However, if the WETH balance of the DecentEthRouter is lower than the transferred amount, the payload is simply dropped and instead, DcntEth is sent to the target address:
https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L266-L269
if (weth.balanceOf(address(this)) < _amount) {
dcntEth.transfer(_to, _amount);
return;
}This could lead to loss of funds as the user may have required the payload to be executed to obtain the funds.
- A user initiates a cross-chain transaction by submitting a message through the
DecentEthRouter.bridgeWithPayload()function. They want to stake WETH on a contract on another chain by calling the functionstakeWETHFor(address account, uint256 amount). - The
DecentEthRouterreceives the message inonOFTReceived(). - Its WETH balance is less than the amount of WETH the user is transferring.
- The payload is dropped and instead transfers
DcntEthdirectly to the target contract. - The user's funds are locked as
DcntEthin the contract and cannot be recovered.
Manual review
If the WETH balance is not sufficient when processing a message of type MT_ETH_TRANSFER_WITH_PAYLOAD, revert and allow the sender to retry the message later. Alternatively, ensure there is always enough WETH in all supported chains, or at the very least document this behavior.
This issue was downgraded from Medium to Low
From the contest docs:
consider scope of blockchains to those supported by layerzero
The DecentEthRouter.onOFTReceived() function is responsible for handling incoming messages from the Decent bridge. It decodes the payload of the transfer to extract necessary information such as the type of message, sender, recipient, and whether the transfer should be in ETH or WETH.
The issue arises in the decoding of the payload. The function assumes that the address size is always 20 bytes. This is true for Ethereum-based chains, but not for all blockchains. For instance, Aptos uses 32-byte addresses. This discrepancy in address size can lead to incorrect decoding of the payload, potentially causing transfers to fail or be misdirected.
- User initiates a transfer from Aptos to an Ethereum-based chain using the bridge.
- The
onOFTReceived()function on the Ethereum side tries to decode the payload. - The function assumes a 20-byte address, but the payload contains a 32-byte Aptos address.
- The decoding fails or produces incorrect results, leading to a failed or misdirected transfer.
Manual review
The contract should not make assumptions about the size of addresses. Instead, it should dynamically determine the correct size based on the chain ID. This could be achieved by maintaining a mapping of chain IDs to address sizes, which can be updated as new chains are supported. The function would then use this mapping to correctly decode the payload. This would ensure compatibility with all supported chains, regardless of their address size.
This issue was downgraded from Medium to Low
From the contest docs:
consider scope of blockchains to those supported by layerzero
Avalanche is also supported by LayerZero, however, the project uses Solidity v0.8.20, which is unsupported by Avalanche: https://github.com/code-423n4/2024-01-decent/blob/main/foundry.toml#L5
solc = "0.8.20"
For more details see code-423n4/2024-01-renft-findings#127.
See code-423n4/2024-01-renft-findings#127
Manual review
It's recommended that v0.8.19 is used instead to maintain compatibility with all blockchains supported by LayerZero.