code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

Signature replay attack for AccessManager._verifyAccess( ) #717

Closed c4-bot-4 closed 8 months ago

c4-bot-4 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/AccessManager.sol#L51 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/AccessManager.sol#L61

Vulnerability details

Impact: An hacker can attacks with signature replay,which can result in gaining unauthorized access to contract functions.

Proof of Concept

We can find the following logic process:AccessManager.grantAccess--->AccessManager._verifyAccess()

But there is signature replay risk in _verifyAccess function.

function _verifyAccess(address wallet, bytes memory signature ) internal view returns (bool) { bytes32 messageHash = keccak256(abi.encodePacked(block.chainid, geoVersion, wallet));

    return SigningTools._verifySignature(messageHash, signature);
    }

Tools Used

vscode foundry

Recommended Mitigation Steps

We can use unique nonces, time-based values, signature verification schemes, and well-audited frameworks like OpenZeppelin, and we can effectively deter signature replay attacks

Assessed type

Other

c4-judge commented 9 months ago

Picodes marked the issue as primary issue

c4-sponsor commented 9 months ago

othernet-global (sponsor) acknowledged

c4-sponsor commented 9 months ago

othernet-global marked the issue as disagree with severity

c4-sponsor commented 9 months ago

othernet-global (sponsor) disputed

c4-sponsor commented 9 months ago

othernet-global marked the issue as agree with severity

othernet-global commented 9 months ago

Users who were included with a valid signature and then excluded will not be able to replay their original signature as the geoVersion increments any time a new region has been excluded.

c4-judge commented 8 months ago

Picodes marked the issue as unsatisfactory: Insufficient proof