xiaoming90 - Signature is malleable

[sherlock-admin4]

xiaoming90

medium

Signature is malleable

Summary

The signature is malleable. When a signature is malleable, it means that it is possible to produce another valid signature for the same message (which also means the same digest).

If the intention is only to allow the creator to acknowledge an edge once, the creator can bypass this restriction because the signature is malleable, and the creator can create another signature to acknowledge an edge again.

Vulnerability Detail

The protocol relies on Solady's SignatureCheckerLib to verify that the signature provided is valid. Once a signature has been successfully verified, it will be marked as used in Line 49 below to prevent users from re-using or replaying the same signature.

The Solady's SignatureCheckerLib warns that the library does not check if a signature is non-malleable.

https://github.com/Vectorized/solady/blob/a34977e56cc1437b7ac07e6356261d2b303da686/src/utils/SignatureCheckerLib.sol#L23

/// WARNING! Do NOT use signatures as unique identifiers:
/// - Use a nonce in the digest to prevent replay attacks on the same contract.
/// - Use EIP-712 for the digest to prevent replay attacks across different chains and contracts.
///   EIP-712 also enables readable signing of typed data for better user safety.
/// This implementation does NOT check if a signature is non-malleable.
library SignatureCheckerLib {

Based on the following code, a creator can only acknowledge an edge once because the digest of the signature to acknowledge an edge will be the same (assuming that data is not in use) every time. After a creator acknowledges an edge, the signature (which also means its digest) will be marked as used in Line 49 below, thus preventing the creator from acknowledging the edge again later using the same signature.

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/graph/TitlesGraph.sol#L49

File: TitlesGraph.sol
39:     /// @notice Modified to check the signature for a proxied acknowledgment.
40:     modifier checkSignature(bytes32 edgeId, bytes calldata data, bytes calldata signature) {
41:         bytes32 digest = _hashTypedData(keccak256(abi.encode(ACK_TYPEHASH, edgeId, data)));
42:         if (
43:             !edges[edgeId].to.creator.target.isValidSignatureNowCalldata(digest, signature)
44:                 || _isUsed[keccak256(signature)]
45:         ) {
46:             revert Unauthorized();
47:         }
48:         _;
49:         _isUsed[keccak256(signature)] = true;
50:     }

Impact

When a signature is malleable, it means that it is possible to produce another valid signature for the same message (which also means the same digest).

If the intention is only to allow the creator to acknowledge an edge once, the creator can bypass this restriction because the signature is malleable, and the creator can create another signature to acknowledge an edge again.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/graph/TitlesGraph.sol#L49

Tool used

Manual Review

Recommendation

Consider verifying the s of the signature is within valid bounds to avoid signature malleability.

if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
	 revert("ECDSA: invalid signature 's' value");
}

[pqseags]

This seems to be a duplicate of #273 @Hash01011122

[WangSecurity]

Planning to make a new issue family with this report as the main one. Planning to add the following duplicated:

• 1337 - Signature malleability allows replay attacks in Titlesgraph when acknowledgeEdge and unacknowledgeEdge #9
• juan - Signature malleability- anyone can acknowledge or unacknowledge an edge from someone else's node #53
• cducrest-brainbot - TitlesGraph's checkSignature() does not properly check replayability #105
• 0x73696d616f - TitlesGraph::checkSignature() modifier is vulnerable to signature malleability, allowing the manipulation of edge.acknowledged #178
• fibonacci - TitlesGraph's acknowledge/unacknowledge edge functions are vulnerable to signature malleability #228
• ZdravkoHr. - TitlesGraph signatures are malleable #345
• ComposableSecurity - Lack of protection from signature malleability #429

[0xjuaan]

@WangSecurity add #130, #10 to that list, they are currently incorrectly dupes of #273 which is a dos/frontrunning issue

[Evert0x]

@WangSecurity add #130, #10 to that list, they are currently incorrectly dupes of #273 which is a dos/frontrunning issue

Thank you!