Non-compliance with EIP712

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-06-krystal-defi/blob/f65b381b258290653fa638019a5a134c4ef90ba8/src/StructHash.sol#L202

Vulnerability details

Impact

Invalid signatures

Proof of Concept

StructHash contract prepares the user signatures in the structure of Krystal functions. One of them is hashing RebalanceConfig struct.

Contract: StructHash.sol

190:     struct RebalanceConfig {
191:         RebalanceCondition rebalanceCondition;
192:         RebalanceAction rebalanceAction;
193:         AutoCompound autoCompound;
194:         bool recurring;
195:     }
196:     function _hash(RebalanceConfig memory obj) private pure returns (bytes32) {
197:         return keccak256(abi.encode(
198:             RebalanceConfig_TYPEHASH,
199:             _hash(obj.rebalanceCondition),
200:             _hash(obj.rebalanceAction),
201:             _hash(obj.autoCompound),
202:  >           obj.recurring 
203:         ));
204:     }

For Line: 202, since the param is a boolean, to ensure compliance with EIP-712, the bool should be explicitly casted to a uint256 when encoding it.

Below is quoted from the EIP page:

The atomic values are encoded as follows: Boolean false and true are encoded as uint256 values 0 and 1 respectively.

Tools Used

Manual Review

Recommended Mitigation Steps

196:     function _hash(RebalanceConfig memory obj) private pure returns (bytes32) {
197:         return keccak256(abi.encode(
198:             RebalanceConfig_TYPEHASH,
199:             _hash(obj.rebalanceCondition),
200:             _hash(obj.rebalanceAction),
201:             _hash(obj.autoCompound),
-                obj.recurring 
+                obj.recurring ? uint256(1) : uint256(0)
203:         ));
204:     }

Assessed type

Other

[3docSec]

Report of sufficient quality

[c4-judge]

3docSec marked the issue as satisfactory

[c4-judge]

3docSec marked the issue as selected for report

[Haupc]

function testDummy() external {
    bool a = false;
    assertEq(abi.encode(a), abi.encode(uint256(0)));
    a = true;
    assertEq(abi.encode(a), abi.encode(uint256(1)));       
}

using test above works with pragma abicoder v2; either. It's seems not a bug

[c4-judge]

3docSec marked the issue as unsatisfactory: Invalid