fix: signature validation (#332)

* fix: signature validation

Corrected signature validation

* make method private

* make method private

* Fix crashes
Removed unsafe usage
Added safety checks

* Fix lint

* Update consent proof text
Updated consent proof text to be more readable

---------

Co-authored-by: Alex Risch <alexrisch@Alexs-MacBook-Pro-2.local>

Package.resolved

@@ -41,8 +41,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/xmtp/libxmtp-swift",
       "state" : {
-        "revision" : "ef7be90fa325f6b087bcf87b95d4c296c6e0a667",
-        "version" : "0.4.4-beta4"
+        "revision" : "f2b73d14be21b64feecfa70c789b3302525975ab",
+        "version" : "0.4.4-beta5"
       }
     },
     {

Sources/XMTPiOS/Conversations.swift

@@ -547,26 +547,26 @@ public actor Conversations {
 	}
 
     private func validateConsentSignature(signature: String, clientAddress: String, peerAddress: String, timestamp: UInt64) -> Bool {
-        let message = Signature.consentProofText(peerAddress: peerAddress, timestamp: timestamp)
-
-        guard let signatureData = Data(hex: signature) else {
-            print("Invalid signature format")
+        // timestamp should be in the past
+        if timestamp > UInt64(Date().timeIntervalSince1970 * 1000) {
             return false
         }
-        var sig = Signature()
-        do {
-            sig = try Signature(serializedData: signatureData)
-        } catch {
-            print("Invalid signature format: \(error)")
+        let thirtyDaysAgo = Date().addingTimeInterval(-30 * 24 * 60 * 60)
+        let thirtyDaysAgoTimestamp = UInt64(thirtyDaysAgo.timeIntervalSince1970 * 1000)
+        if timestamp < thirtyDaysAgoTimestamp {
             return false
         }
-        // Convert the message to Data
-        guard let messageData = message.data(using: .utf8) else {
-            print("Invalid message format")
+
+        let message = Signature.consentProofText(peerAddress: peerAddress, timestamp: timestamp)
+
+        guard let signatureData = Data(hex: signature) else {
+            print("Invalid signature format")
             return false
         }
+
         do {
-            let recoveredKey = try KeyUtilx.recoverPublicKeyKeccak256(from: sig.rawData, message: messageData)
+            let ethMessage = try Signature.ethHash(message)
+            let recoveredKey = try KeyUtilx.recoverPublicKey(message: ethMessage, signature: signatureData)
             let address = KeyUtilx.generateAddress(from: recoveredKey).toChecksumAddress()
 
             return clientAddress == address

Sources/XMTPiOS/KeyUtil.swift

@@ -89,6 +89,9 @@ enum KeyUtilx {
 		// get recoverable signature
 		let signaturePtr = UnsafeMutablePointer<secp256k1_ecdsa_recoverable_signature>.allocate(capacity: 1)
 		defer { signaturePtr.deallocate() }
+    if signature.count < 65 {
+        throw KeyUtilError.signatureParseFailure
+    }
 
 		let serializedSignature = Data(signature[0 ..< 64])
 		var v = Int32(signature[64])
@@ -100,28 +103,40 @@ enum KeyUtilx {
 			v -= 35
 		}
 
-		// swiftlint:disable force_unwrapping
-		try serializedSignature.withUnsafeBytes {
-			guard secp256k1_ecdsa_recoverable_signature_parse_compact(ctx, signaturePtr, $0.bindMemory(to: UInt8.self).baseAddress!, v) == 1 else {
-				throw KeyUtilError.signatureParseFailure
-			}
-		}
-		let pubkey = UnsafeMutablePointer<secp256k1_pubkey>.allocate(capacity: 1)
+    try serializedSignature.withUnsafeBytes {
+        guard let baseAddress = $0.bindMemory(to: UInt8.self).baseAddress else {
+            throw KeyUtilError.signatureParseFailure // or a more specific error for "empty buffer"
+        }
+        if v > 3 {
+            throw KeyUtilError.signatureParseFailure
+        }
+        guard secp256k1_ecdsa_recoverable_signature_parse_compact(ctx, signaturePtr, baseAddress, v) == 1 else {
+            throw KeyUtilError.signatureParseFailure
+        }
+    }
+
+    let pubkey = UnsafeMutablePointer<secp256k1_pubkey>.allocate(capacity: 1)
 		defer { pubkey.deallocate() }
 
-		try message.withUnsafeBytes {
-			guard secp256k1_ecdsa_recover(ctx, pubkey, signaturePtr, $0.bindMemory(to: UInt8.self).baseAddress!) == 1 else {
-				throw KeyUtilError.signatureFailure
-			}
-		}
+        try message.withUnsafeBytes {
+            guard let baseAddress = $0.bindMemory(to: UInt8.self).baseAddress else {
+                throw KeyUtilError.signatureFailure // Consider throwing a more specific error
+            }
+
+            guard secp256k1_ecdsa_recover(ctx, pubkey, signaturePtr, baseAddress) == 1 else {
+                throw KeyUtilError.signatureFailure
+            }
+        }
 
 		var size = 65
-		var rv = Data(count: size)
-		_ = rv.withUnsafeMutableBytes {
-			secp256k1_ec_pubkey_serialize(ctx, $0.bindMemory(to: UInt8.self).baseAddress!, &size, pubkey, UInt32(SECP256K1_EC_UNCOMPRESSED))
-		}
+        var rv = Data(count: size)
+        rv.withUnsafeMutableBytes { buffer -> Void in
+            guard let baseAddress = buffer.bindMemory(to: UInt8.self).baseAddress else {
+                return // Optionally, handle the error or log this condition
+            }
+            secp256k1_ec_pubkey_serialize(ctx, baseAddress, &size, pubkey, UInt32(SECP256K1_EC_UNCOMPRESSED))
+        }
 
-		// swiftlint:enable force_unwrapping
 		return rv
 	}
 }

Sources/XMTPiOS/Messages/Signature.swift

@@ -64,10 +64,16 @@ extension Signature {
 	}
 
     static func consentProofText(peerAddress: String, timestamp: UInt64) -> String {
+        let date = Date(timeIntervalSince1970: Double(timestamp) / 1000)
+        let dateFormatter = DateFormatter()
+        dateFormatter.timeZone = TimeZone(abbreviation: "UTC")
+        dateFormatter.dateFormat = "EEE, d MMM yyyy HH:mm:ss 'GMT'"
+        let dateString = dateFormatter.string(from: date)
+
         return (
             "XMTP : Grant inbox consent to sender\n" +
             "\n" +
-            "Current Time: \(timestamp)\n" +
+            "Current Time: \(dateString)\n" +
             "From Address: \(peerAddress)\n" +
             "\n" +
             "For more info: https://xmtp.org/signatures/"

Tests/XMTPTests/ConversationsTest.swift

@@ -204,9 +204,9 @@ class ConversationsTests: XCTestCase {
 
         let timestamp = UInt64(Date().timeIntervalSince1970 * 1000)
         let signatureText = Signature.consentProofText(peerAddress: boClient.address, timestamp: timestamp)
-        let digest = Data(signatureText.utf8)
-        let signature = try await alix.sign(Util.keccak256(digest))
-        let hex = Data(try signature.serializedData()).toHex
+        let signature = try await alix.sign(message: signatureText)
+
+        let hex = signature.rawData.toHex
         var consentProofPayload = ConsentProofPayload()
         consentProofPayload.signature = hex
         consentProofPayload.timestamp = timestamp
@@ -231,9 +231,8 @@ class ConversationsTests: XCTestCase {
 
         let timestamp = UInt64(Date().timeIntervalSince1970 * 1000)
         let signatureText = Signature.consentProofText(peerAddress: boClient.address, timestamp: timestamp)
-        let digest = Data(signatureText.utf8)
-        let signature = try await alix.sign(Util.keccak256(digest))
-        let hex = Data(try signature.serializedData()).toHex
+        let signature = try await alix.sign(message: signatureText)
+        let hex = signature.rawData.toHex
         var consentProofPayload = ConsentProofPayload()
         consentProofPayload.signature = hex
         consentProofPayload.timestamp = timestamp
@@ -259,9 +258,8 @@ class ConversationsTests: XCTestCase {
 
         let timestamp = UInt64(Date().timeIntervalSince1970 * 1000)
         let signatureText = Signature.consentProofText(peerAddress: boClient.address, timestamp: timestamp + 1)
-        let digest = Data(signatureText.utf8)
-        let signature = try await alix.sign(Util.keccak256(digest))
-        let hex = Data(try signature.serializedData()).toHex
+        let signature = try await alix.sign(message:signatureText)
+        let hex = signature.rawData.toHex
         var consentProofPayload = ConsentProofPayload()
         consentProofPayload.signature = hex
         consentProofPayload.timestamp = timestamp

Tests/XMTPTests/SignatureTests.swift

@@ -16,4 +16,13 @@ class SignatureTests: XCTestCase {
 		let signature = try await signingKey.sign(Data(digest))
 		XCTAssert(try signature.verify(signedBy: signingKey.publicKey, digest: Data("Hello world".utf8)))
 	}
+
+    func testConsentProofText() {
+        let timestamp = UInt64(1581663600000)
+        let exampleAddress = "0x1234567890abcdef";
+        let text = Signature.consentProofText(peerAddress: exampleAddress, timestamp: timestamp)
+        let expected = "XMTP : Grant inbox consent to sender\n\nCurrent Time: Fri, 14 Feb 2020 07:00:00 GMT\nFrom Address: 0x1234567890abcdef\n\nFor more info: https://xmtp.org/signatures/"
+
+        XCTAssertEqual(text, expected)
+    }
 }