hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

0 stars 1 forks source link

nonce is converted to bytes32 twice in `_encryptPayload()` of `RotatingKeys.sol` #39

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x861315d0d4b0363878a2ac31c6228ce1dfb19a9aeba7695e5039edbb6dc4e356 Severity: low

Description: Description\

In RotatingKeys.sol, _encryptPayload() is implemented as:

    function _encryptPayload(bytes memory payload) internal view returns (bytes memory encryptedData, uint256 keyIndex) {
        require(_ringKeys.length > 0, "No ring keys set up");

        keyIndex = _ringKeys.length - 1;
@>        bytes32 nonce = _computeNonce(keyIndex);           @audit // nonce is already returned as bytes32
        encryptedData = Sapphire.encrypt(_ringKeys[keyIndex], bytes32(nonce), payload, abi.encodePacked(nonceConst));
    }

While encrypting data, it passes nonce as bytes32 argument to Sapphire.encrypt() function. However, nonce is already a bytes32 value returned from _computeNonce() function.

    function _computeNonce(uint256 keyIndex) private view returns (bytes32 nonce) {
        nonce = keccak256(abi.encodePacked(keyIndex, nonceConst));
    }

So, again converting nonce to bytes32 is not necessary.

Recommendations\

Consider below changes:

    function _encryptPayload(bytes memory payload) internal view returns (bytes memory encryptedData, uint256 keyIndex) {
        require(_ringKeys.length > 0, "No ring keys set up");

        keyIndex = _ringKeys.length - 1;
        bytes32 nonce = _computeNonce(keyIndex);
-        encryptedData = Sapphire.encrypt(_ringKeys[keyIndex], bytes32(nonce), payload, abi.encodePacked(nonceConst));
+         encryptedData = Sapphire.encrypt(_ringKeys[keyIndex], nonce, payload, abi.encodePacked(nonceConst));
    }
0xRizwan commented 4 months ago

@party-for-illuminati Can you please furnish the invalidation reason?

party-for-illuminati commented 4 months ago

@party-for-illuminati Can you please furnish the invalidation reason?

Informational