DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: medium
Invalid

Lack of chainID validation allows signatures to be re-used in events of forks

Summary

There are two custom libraries LibSiloPermit.sol and LibTokenPermit.sol that implement permit functions, which are basically gasless approvals via signatures, an owner sets allowance to a spender via signature. However these signatures doesn't account for chain forks.

Vulnerability Details

As can be seen the chain id is included in the "domain separator":

function _domainSeparatorV4() internal view returns (bytes32) {
return
_buildDomainSeparator(
DEPOSIT_PERMIT_EIP712_TYPE_HASH,
DEPOSIT_PERMIT_HASHED_NAME,
DEPOSIT_PERMIT_HASHED_VERSION
);
}
function _buildDomainSeparator(
bytes32 typeHash,
bytes32 name,
bytes32 version
) internal view returns (bytes32) {
!!!!!!!!!!!!!
return keccak256(abi.encode(typeHash, name, version, C.getChainId(), address(this)));
}

However it's not included in the signed data as part of the permit call:

function permit(
address owner,
address spender,
address token,
uint256 value,
uint256 deadline,
uint8 v,
bytes32 r,
bytes32 s
) internal {
...
bytes32 structHash = keccak256(
abi.encode(
DEPOSIT_PERMIT_TYPEHASH,
owner,
spender,
token,
value,
_useNonce(owner),
deadline
)
);

So in case of a chain fork after deployment, the signed message may still be valid on both forks.

Here is a scenario:

  1. The approval is included in an upcoming hard fork that will separate the community.

  2. Some users stay on the old chain, some on the new chain.

  3. On the new chain Alice approves Bob to spend some tokens via permit().

  4. Bob being on both chains, replays the permit() call on the old chain and this allows him to steal value.

Impact

  • Impact: High, as there is a chance for a potential theft

  • Likelihood: Low, as it requires a fork to occur

  • Overall: Medium

Tools Used

Manual Review

Recommendations

Condsider dynamically computing the chain id inside the functions and then add it in the signature schemas:

uint256 chainid;
assembly {
chainid := chainid()
Add in the computed chain id
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Replay attack in case of hard fork - Hardcoded chainId 712

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.