KlimaToken Manual Review Findings

KTN-01M: Cross-Chain Replay Attack

Description:

The DOMAIN_SEPARATOR value is calculated only once during the contract's constructor, a practice that is advised against by the EIP-2612 standard.

Example:

Copy
1103constructor() {
1104    uint256 chainID;
1105    assembly {
1106        chainID := chainid()
1107    }
1108
1109    DOMAIN_SEPARATOR = keccak256(
1110        abi.encode(
1111            keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
1112            keccak256(bytes(name())),
1113            keccak256(bytes("1")), // Version
1114            chainID,
1115            address(this)
1116        )
1117    );
1118}

Recommendation:

We advise it to either be reconstructed on each invocation or to utilize a caching system similar to that of the draft-EIP712 implementation of OpenZeppelin.

Alleviation:

The KlimaDAO team responded by stating that this is a negligible risk as the network the token is deployed in would need to be compromised to perform the fork.

KTN-02M: Insecure Elliptic Curve Recovery Mechanism

Description:

The ecrecover function is a low-level cryptographic function that should be utilized after appropriate sanitizations have been enforced on its arguments, namely on the s and v values. This is due to the inherent trait of the curve to be symmetrical on the x-axis and thus permitting signatures to be replayed with the same x value (r) but a different y value (s).

Example:

Copy
1124function permit(
1125    address owner,
1126    address spender,
1127    uint256 amount,
1128    uint256 deadline,
1129    uint8 v,
1130    bytes32 r,
1131    bytes32 s
1132) public virtual override {
1133    require(block.timestamp <= deadline, "Permit: expired deadline");
1134
1135    bytes32 hashStruct =
1136    keccak256(abi.encode(PERMIT_TYPEHASH, owner, spender, amount, _nonces[owner].current(), deadline));
1137
1138    bytes32 _hash = keccak256(abi.encodePacked(uint16(0x1901), DOMAIN_SEPARATOR, hashStruct));
1139
1140    address signer = ecrecover(_hash, v, r, s);
1141    require(signer != address(0) && signer == owner, "ZeroSwapPermit: Invalid signature");
1142
1143    _nonces[owner].increment();
1144    _approve(owner, spender, amount);
1145}

Recommendation:

We advise them to be sanitized by ensuring that v is equal to either 27 or 28 (v ∈ {27, 28}) and to ensure that s is existent in the lower half order of the elliptic curve (0 < s < secp256k1n ÷ 2 + 1) by ensuring it is less than 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A1. A reference implementation of those checks can be observed in the ECDSA library of OpenZeppelin and the rationale behind those restrictions exists within Appendix F of the Yellow Paper.

Alleviation:

The KlimaDAO team opted not to apply on-chain sanitization as it is gas intensive.

KTN-03M: Overly Centralized Functionality

Description:

The vault ownership system is completely managed by the owner of the contract, in turn allowing the owner to change the minter of new units and thus completely compromise the token.

Example:

Copy
1319function mint(address account_, uint256 amount_) external onlyVault() {
1320    _mint(account_, amount_);
1321}

Recommendation:

We advise this trait of the system to be carefully evaluated as it introduces significant centralization to the overall project.

Alleviation:

The KlimaDAO team evaluated this exhibit and stated that the ownership of the contract is guarded by a multisignature wallet and as such no change will be made to the code.