Incorrect address returned from signedPrefixedMessageToKey when invalid data passed in

[kibagateaux]

Incorrect address returned from signedPrefixedMessageToKey when invalid data passed in

Steps To Reproduce

There are two ways to get incorrect address returned from signedPrefixedMessageToKey

1. invalid signature hash
2. invalid message

public static String getAddressUsedToSignHashedMessage(String signedMessageInHex, String originalMessage)
   throws SignatureException {
        if (signedMessageInHex.startsWith("0x")) {
            signedMessageInHex = signedMessageInHex.substring(2);
        }

        // No need to prepend these strings with 0x because
        // Numeric.hexStringToByteArray() accepts both formats
        String r = signedMessageInHex.substring(0, 64);
        String s = signedMessageInHex.substring(64, 128);
        String v = signedMessageInHex.substring(128, 130);

        // Using Sign.signedPrefixedMessageToKey for EIP-712 compliant signatures.
        String pubkey = Sign.signedPrefixedMessageToKey(originalMessage.getBytes(),
                new Sign.SignatureData(
                        Numeric.hexStringToByteArray(v)[0],
                        Numeric.hexStringToByteArray(r),
                        Numeric.hexStringToByteArray(s)))
                .toString(16);

        return Keys.getAddress(pubkey);
}
    
Sign.SignatureData signatureData = Sign.signPrefixedMessage(myMsg.getBytes());
byte[] signedMessageBytes = new byte[32 + 32 + 1]; // 32 bytes for r, 32 bytes for s, 1 byte for v
System.arraycopy(signatureData.getR(), 0, signedMessageBytes, 0, 32);
System.arraycopy(signatureData.getS(), 0, signedMessageBytes, 32, 32);
System.arraycopy(signatureData.getV(), 0, signedMessageBytes, 64, 1);
String mySignedMessageHex = Numeric.toHexString(signedMessageBytes);

// Example #1 - Invalid Sig 
String  invalidAddress1 = getAddressUsedToSignHashedMessage(yourRandomSignedMessageHex, myMsg.getBytes())

// Example #2 - Invalid message
String  aDifferentInvalidAddress = getAddressUsedToSignHashedMessage(mySignedMessageHex, yourRandomMsg.getBytes())

// Working Example 
String  myAddress = getAddressUsedToSignHashedMessage(mySignedMessageHex, myMsg.getBytes())

Expected behavior

signedPrefixedMessageToKey returns null if:

1. Signature data is compromised in some way
2. message passed does not match original signature
3. its not a valid ETH signature at all

Actual behavior

When invalid data passed to signedPrefixedMessageToKey I get random addresses returned that did not sign the message and are not on the wallet that signed the original message.

Environment

• Web3j version: 4.10.3
• Java version : openjdk 18.0.2 + using Clojure version 1.11.1
• Operating System 1: macOS 13.0.1 with M1 chip

Additional context

• This isn't how ecrecover works in Solidity so it was unexpected behaviour to me but maybe it is intentional for some reason?
• Would be great to know how I can programmatically check if the signature is valid or not if signedPrefixedMessageToKey is supposed to be returning random addresses using invalid data
• Very critical bug since signedPrefixedMessageToKey assumes EIP-712 signatures so a valid ETH signature from the right address could be passed in but with the wrong format and web3j would return the wrong signer. So someone might think this function does work on non EIP-712 signatures and use incorrectly.
• I know the function docs say to comapre returned address from expected signer but that feels incorrect to me because then the logic isnt entirely based on cryptography. Someone could somehow figure out the false address that will be returned from an invalid signature to bypass any checks or worse figure out how to spoof a specific address.
• Could be issue with me coercing v value to 27/28 except that i checked my signed message and it uses 27/28 already. Also this should still work according to EIP-712 spec I'm pretty sure.
• I dont believe that this has to do with Java interop from Clojure since all code is working fine when correct parameters provided.
• I've been using this Java gist as reference for my Clojure code.
• Clojure code implementing ecrecover that is returning false addresses

(defn ecrecover
    "original-msg is human readable string that signer was shown
    signed-msg-hash is hexstring bytecode output of rpc/wallet signing function
    returns checksummed ethereum address that signed msg-hash
    "
    [signed-msg-hash original-msg]
    (let [raw-hex-str (if (clojure.string/starts-with? signed-msg-hash "0x")
                        (subs signed-msg-hash 2)
                        signed-msg-hash)
        r (hex->bytes (subs raw-hex-str 0 64))
        s (hex->bytes (subs raw-hex-str 64 128))
        _v (subs raw-hex-str 128 130) ;; could be 0/1 or 27/28
        v (hex->bytes (if (< (hex->int _v) 27) (int->hex (+ (hex->int _v) 27)) _v)) ;; so coerce to ETH native 27/28
        signature-data (new org.web3j.crypto.Sign$SignatureData (first v) r s)
        hashed-msg (.getBytes original-msg)
        ;; Using Sign.signedPrefixedMessageToKey for EIP-712 compliant signatures
        pubkey (Sign/signedPrefixedMessageToKey hashed-msg signature-data)
        address (if (nil? pubkey) nil (Keys/toChecksumAddress (Keys/getAddress (bigint->hex pubkey))))]
    address))