checkSignatureValidity() verification by signature do not utilize nonces and can be tricked by using owner / builder signatures from earlier calls. Namely, while checkSignatureValidityโs approvedHashes based way can used only once as it deletes the corresponding array entry, the _signature based logic can be reused and itself contains no nonce, just validating that the signature for the constant message is from the desired actor. This validation will go through for all the subsequent calls, which provides a way to bypass the verification by supplying previously recorded signature for the same hash used in a different operation.
Signatory replay is a low level issue that opens up a range of attack surfaces. Currently all the uses besides escrow() utilize nonces, so the impact consists of escrow() calls being repeated as there is no nonce in the data, reducing the debt up to zero.
escrow() doesnโt has nonce in the _hash, so can be rerun with the same _data:
function escrow(bytes calldata _data, bytes calldata _signature)
external
virtual
override
whenNotPaused
{
// Decode params from _data
(
uint256 _communityID,
address _builder,
address _lender,
address _agent,
address _project,
uint256 _repayAmount,
bytes memory _details
) = abi.decode(
_data,
(uint256, address, address, address, address, uint256, bytes)
);
// Compute hash from bytes
bytes32 _hash = keccak256(_data);
// Local instance of variable. For saving gas.
IProject _projectInstance = IProject(_project);
// Revert if decoded builder is not decoded project's builder
require(_builder == _projectInstance.builder(), "Community::!Builder");
// Revert if decoded _communityID's owner is not decoded _lender
require(
_lender == _communities[_communityID].owner,
"Community::!Owner"
);
// check signatures
checkSignatureValidity(_lender, _hash, _signature, 0); // must be lender
checkSignatureValidity(_builder, _hash, _signature, 1); // must be builder
checkSignatureValidity(_agent, _hash, _signature, 2); // must be agent or escrow
// Internal call to reduce debt
_reduceDebt(_communityID, _project, _repayAmount, _details);
emit DebtReducedByEscrow(_agent);
}
checkSignatureValidity() allows two ways of verification, where approvedHashes based way can used only once, while _signature based approach can be reused repeatedly:
function checkSignatureValidity(
address _address,
bytes32 _hash,
bytes memory _signature,
uint256 _signatureIndex
) internal virtual {
// Decode signer
address _recoveredSignature = SignatureDecoder.recoverKey(
_hash,
_signature,
_signatureIndex
);
// Revert if decoded signer does not match expected address
// Or if hash is not approved by the expected address.
require(
_recoveredSignature == _address || approvedHashes[_address][_hash],
"Community::invalid signature"
);
// Delete from approvedHash. So that signature cannot be reused.
delete approvedHashes[_address][_hash];
}
This holds as SignatureDecoderโs recoverKey() can be run with the same signature, there is no nonce, so it can be run for the same messageHash:
function recoverKey(
bytes32 messageHash,
bytes memory messageSignatures,
uint256 pos
) internal pure returns (address) {
if (messageSignatures.length % 65 != 0) {
return (address(0));
}
uint8 v;
bytes32 r;
bytes32 s;
(v, r, s) = signatureSplit(messageSignatures, pos);
// If the version is correct return the signer address
if (v != 27 && v != 28) {
return (address(0));
} else {
// solium-disable-next-line arg-overflow
return ecrecover(toEthSignedMessageHash(messageHash), v, r, s);
}
}
To target the root source nonce has to be presented in all the signature verifications, invalidating all the subsequent uses.
This way consider ensuring that in all the instances checkSignatureValidity() is called for the hash that includes a specific nonce for the functionality. Whenever this doesnโt hold the signature replay is possible with full consequences being reasonably hard to track.
The text was updated successfully, but these errors were encountered:
๐ 1 horsefacts reacted with thumbs up emoji ๐ 1 horsefacts reacted with eyes emoji
All reactions