You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Some parameters that are used in the merkle tree proofs are malleable for certain parts of the proof.
The serializeConsensusHeader function in the FuelERC20Gateway truncates the header.height of type uint64 to a uint32.
Figure 7.1: The serializeConsensusHeader function in fuel-v2-contracts/contracts/fuelchain/types/FuelBlockHeader.sol
function serializeConsensusHeader(FuelBlockHeader memory header) internal pure returns (bytes memory) {
return
abi.encodePacked(
header.prevRoot,
(uint32)(header.height),
header.timestamp,
computeApplicationHeaderHash(header)
);
}
This means that the upper bits are malleable and changing them will result in the same hash. We were, however, not able to identify an attack vector.
In a similar manner, message.recipient is being truncated from bytes32 to address (20 bytes) in _executeMessage.
Figure 7.2: Snippet from the _executeMessage function in fuel-v2-contracts/contracts/fuelchain/FuelMessagePortal.sol
In this case, changing the recipient bits would change the messageId required for the inclusion proof, meaning that this is not vulnerable in its current form.
We highlight these cases nonetheless, because changing code requirements and specifications can lead to mistakes.
Recommendations
Short term, do not truncate header.height and check that the upper bits for the message.recipient are zero.
Long term, aim to minimize any degrees of freedom that a user has on the system that could lead to a vulnerability.
The text was updated successfully, but these errors were encountered:
Closes#56
After checking possible side effects, I could only come up with the fact
that `relayMessage` - the function used by users to withdraw from L2 to
L1 - will change its function signature as its input type will be
changed. This will warrant the change in whatever frontend that might be
using this function.
This change also brings the solidity typings closer to the [fuel-core
typings](https://github.com/FuelLabs/fuel-core/blob/7934cbd05900169a7ed24f78e09fdab04718ec74/crates/types/src/blockchain/header.rs#L91C23-L91C28),
as `height` is an `uint32` type in the consensus. I am unsure about the
reasoning behind it being `uint64` in solidity, just to truncate it
later to `uint32`. Was it originally `u64` in the consensus spec? Maybe
@Voxelot can bring some light into it.
EDIT. [So it seems that this is a remnant from a change of
spec](FuelLabs/fuel-specs@667429c)
Description
Some parameters that are used in the merkle tree proofs are malleable for certain parts of the proof.
The serializeConsensusHeader function in the FuelERC20Gateway truncates the header.height of type uint64 to a uint32.
Figure 7.1: The serializeConsensusHeader function in fuel-v2-contracts/contracts/fuelchain/types/FuelBlockHeader.sol
This means that the upper bits are malleable and changing them will result in the same hash. We were, however, not able to identify an attack vector.
In a similar manner, message.recipient is being truncated from bytes32 to address (20 bytes) in _executeMessage.
Figure 7.2: Snippet from the _executeMessage function in fuel-v2-contracts/contracts/fuelchain/FuelMessagePortal.sol
In this case, changing the recipient bits would change the messageId required for the inclusion proof, meaning that this is not vulnerable in its current form.
We highlight these cases nonetheless, because changing code requirements and specifications can lead to mistakes.
Recommendations
Short term, do not truncate header.height and check that the upper bits for the message.recipient are zero.
Long term, aim to minimize any degrees of freedom that a user has on the system that could lead to a vulnerability.
The text was updated successfully, but these errors were encountered: