From 0599b98e307888b2d78da1505a0e99be09ba59d9 Mon Sep 17 00:00:00 2001 From: Nalin Bhardwaj Date: Thu, 16 May 2024 16:59:00 -0700 Subject: [PATCH] WebAuthn.sol: simplify contains to startsWith --- lcov.info | 267 +++++++++++++++++++--------------------- src/WebAuthn.sol | 36 +++--- test/P256Verifier.t.sol | 17 +-- test/WebAuthn.t.sol | 58 +++------ 4 files changed, 169 insertions(+), 209 deletions(-) diff --git a/lcov.info b/lcov.info index 47c8e9a..95863d0 100644 --- a/lcov.info +++ b/lcov.info @@ -428,134 +428,125 @@ BRH:39 end_of_record TN: SF:src/WebAuthn.sol -FN:12,WebAuthn.contains -FNDA:11,WebAuthn.contains -DA:17,11 -DA:17,11 -DA:17,11 -DA:18,11 -DA:18,11 -DA:18,11 -DA:20,11 -DA:20,11 -DA:21,11 -DA:21,11 -DA:23,11 -DA:23,11 -DA:23,176 -DA:23,176 -DA:24,168 -DA:24,168 -DA:24,168 -BRDA:24,0,0,167 -BRDA:24,0,1,1 -DA:25,1 -DA:25,1 -DA:28,167 -DA:28,167 -BRDA:28,1,0,165 -BRDA:28,1,1,2 -DA:29,2 -DA:29,2 -DA:33,8 -DA:33,8 -FN:44,WebAuthn.checkAuthFlags +FN:12,WebAuthn.startsWith +FNDA:6,WebAuthn.startsWith +DA:16,6 +DA:16,6 +DA:16,6 +DA:17,6 +DA:17,6 +DA:17,6 +DA:19,6 +DA:19,6 +DA:20,6 +DA:20,6 +DA:22,6 +DA:22,6 +DA:22,212 +DA:22,212 +DA:23,209 +DA:23,209 +BRDA:23,0,0,208 +BRDA:23,0,1,1 +DA:24,1 +DA:24,1 +DA:27,208 +DA:27,208 +BRDA:27,1,0,206 +BRDA:27,1,1,2 +DA:28,2 +DA:28,2 +DA:32,3 +DA:32,3 +FN:43,WebAuthn.checkAuthFlags FNDA:9,WebAuthn.checkAuthFlags -DA:49,9 -DA:49,9 -DA:49,9 -BRDA:49,2,0,8 -BRDA:49,2,1,1 -DA:50,1 -DA:50,1 -DA:57,8 -DA:57,8 -DA:58,2 -DA:58,2 -BRDA:56,3,0,7 -BRDA:56,3,1,1 -DA:60,1 -DA:60,1 -DA:65,7 -DA:65,7 -DA:65,7 -BRDA:65,4,0,1 -BRDA:65,4,1,1 -DA:66,2 -DA:66,2 -DA:66,2 -BRDA:66,5,0,1 -BRDA:66,5,1,1 -DA:67,1 -DA:67,1 -DA:71,6 -DA:71,6 -FN:124,WebAuthn.verifySignature +DA:48,9 +DA:48,9 +DA:48,9 +BRDA:48,2,0,8 +BRDA:48,2,1,1 +DA:49,1 +DA:49,1 +DA:56,8 +DA:56,8 +DA:57,5 +DA:57,5 +BRDA:55,3,0,7 +BRDA:55,3,1,1 +DA:59,1 +DA:59,1 +DA:64,7 +DA:64,7 +DA:64,7 +BRDA:64,4,0,1 +BRDA:64,4,1,1 +DA:65,2 +DA:65,2 +DA:65,2 +BRDA:65,5,0,1 +BRDA:65,5,1,1 +DA:66,1 +DA:66,1 +DA:70,6 +DA:70,6 +FN:123,WebAuthn.verifySignature FNDA:10,WebAuthn.verifySignature -DA:138,10 -DA:138,10 -DA:138,10 -DA:139,9 -DA:139,9 -BRDA:137,6,0,6 -BRDA:137,6,1,4 -DA:141,4 -DA:141,4 +DA:135,10 +DA:135,10 +DA:135,10 +DA:136,9 +DA:136,9 +BRDA:134,6,0,6 +BRDA:134,6,1,4 +DA:138,4 +DA:138,4 +DA:144,6 +DA:144,6 +DA:144,6 +DA:145,6 DA:145,6 DA:145,6 -DA:146,6 -DA:146,6 -BRDA:146,7,0,5 -BRDA:146,7,1,1 -DA:147,1 -DA:147,1 -DA:151,5 -DA:151,5 -DA:151,5 -DA:152,5 -DA:152,5 -DA:152,5 -DA:158,5 -DA:158,5 -BRDA:158,8,0,3 -BRDA:158,8,1,2 -DA:159,2 -DA:159,2 -DA:163,3 -DA:163,3 -DA:163,3 -DA:164,3 -DA:164,3 -DA:164,3 -DA:168,3 -DA:168,3 -DA:168,3 +DA:150,6 +DA:150,6 +BRDA:150,7,0,3 +BRDA:150,7,1,3 +DA:151,3 +DA:151,3 +DA:155,3 +DA:155,3 +DA:155,3 +DA:156,3 +DA:156,3 +DA:156,3 +DA:160,3 +DA:160,3 +DA:160,3 FNF:3 FNH:3 -LF:32 -LH:32 -BRF:18 -BRH:18 +LF:29 +LH:29 +BRF:16 +BRH:16 end_of_record TN: SF:src/utils/Base64URL.sol FN:7,Base64URL.encode -FNDA:1799,Base64URL.encode -DA:8,1799 -DA:8,1799 -DA:8,1799 -DA:9,1799 -DA:9,1799 -DA:9,1799 -DA:12,1799 -DA:12,1799 -DA:13,1799 -DA:13,1799 -DA:13,1799 -DA:13,1795 -BRDA:13,0,0,518 +FNDA:1800,Base64URL.encode +DA:8,1800 +DA:8,1800 +DA:8,1800 +DA:9,1800 +DA:9,1800 +DA:9,1800 +DA:12,1800 +DA:12,1800 +DA:13,1800 +DA:13,1800 +DA:13,1800 +DA:13,1796 +BRDA:13,0,0,519 BRDA:13,0,1,1281 -DA:13,518 +DA:13,519 DA:14,1281 DA:14,1281 DA:14,1281 @@ -563,33 +554,33 @@ DA:14,1277 BRDA:14,1,0,1281 BRDA:14,1,1,511 DA:14,511 -DA:16,1799 -DA:16,1799 -DA:16,1799 -DA:17,1799 -DA:17,1799 -DA:17,1799 -DA:19,1799 -DA:19,1799 -DA:19,526644 -DA:19,526644 -DA:20,524845 -DA:20,524845 +DA:16,1800 +DA:16,1800 +DA:16,1800 +DA:17,1800 +DA:17,1800 +DA:17,1800 +DA:19,1800 +DA:19,1800 +DA:19,526651 +DA:19,526651 +DA:20,524851 +DA:20,524851 BRDA:20,2,0,1248 -BRDA:20,2,1,523597 +BRDA:20,2,1,523603 DA:21,1248 DA:21,1248 -DA:22,523597 -DA:22,523597 +DA:22,523603 +DA:22,523603 BRDA:22,3,0,176191 -BRDA:22,3,1,347406 +BRDA:22,3,1,347412 DA:23,176191 DA:23,176191 -DA:25,347406 -DA:25,347406 -DA:29,1799 -DA:29,1799 -DA:29,1799 +DA:25,347412 +DA:25,347412 +DA:29,1800 +DA:29,1800 +DA:29,1800 FNF:1 FNH:1 LF:14 diff --git a/src/WebAuthn.sol b/src/WebAuthn.sol index e616eb3..b7fefbd 100644 --- a/src/WebAuthn.sol +++ b/src/WebAuthn.sol @@ -8,24 +8,23 @@ import "./P256.sol"; * Helper library for external contracts to verify WebAuthn signatures. **/ library WebAuthn { - /// Checks whether substr occurs in str starting at a given byte offset. - function contains( - string memory substr, - string memory str, - uint256 location + /// Checks whether prefix occurs in the beginning of str. + function startsWith( + string memory prefix, + string memory str ) internal pure returns (bool) { - bytes memory substrBytes = bytes(substr); + bytes memory prefixBytes = bytes(prefix); bytes memory strBytes = bytes(str); - uint256 substrLen = substrBytes.length; + uint256 prefixLen = prefixBytes.length; uint256 strLen = strBytes.length; - for (uint256 i = 0; i < substrLen; i++) { - if (location + i >= strLen) { + for (uint256 i = 0; i < prefixLen; i++) { + if (i >= strLen) { return false; } - if (substrBytes[i] != strBytes[location + i]) { + if (prefixBytes[i] != strBytes[i]) { return false; } } @@ -126,8 +125,6 @@ library WebAuthn { bytes memory authenticatorData, bool requireUserVerification, string memory clientDataJSON, - uint256 challengeLocation, - uint256 responseTypeLocation, uint256 r, uint256 s, uint256 x, @@ -142,20 +139,15 @@ library WebAuthn { } // Check that response is for an authentication assertion - string memory responseType = '"type":"webauthn.get"'; - if (!contains(responseType, clientDataJSON, responseTypeLocation)) { - return false; - } - - // Check that challenge is in the clientDataJSON + // and that the challenge is in the clientDataJSON + // as per https://www.w3.org/TR/webauthn-2/#clientdatajson-serialization string memory challengeB64url = Base64URL.encode(challenge); - string memory challengeProperty = string.concat( - '"challenge":"', + string memory prefix = string.concat( + '{"type":"webauthn.get","challenge":"', challengeB64url, '"' ); - - if (!contains(challengeProperty, clientDataJSON, challengeLocation)) { + if (!startsWith(prefix, clientDataJSON)) { return false; } diff --git a/test/P256Verifier.t.sol b/test/P256Verifier.t.sol index 0147691..3a11c48 100644 --- a/test/P256Verifier.t.sol +++ b/test/P256Verifier.t.sol @@ -123,35 +123,38 @@ contract P256VerifierTest is Test { // In-bounds dummy key (1, 1) // Calls modexp, which takes gas. - (bool result, uint gasUsed) = evaluate(hash, r, s, x, y); - console2.log("gasUsed ", gasUsed); + (bool result, uint gasUsedWithModexp) = evaluate(hash, r, s, x, y); + console2.log("gasUsed ", gasUsedWithModexp); assertEq(result, false); - assertGt(gasUsed, 1500); + assertGt(gasUsedWithModexp, 5000); + + uint failFastGasBound = (gasUsedWithModexp * 9) / 10; // Out-of-bounds public key. Fails fast, takes less gas. (x, y) = (0, 1); + uint gasUsed; (result, gasUsed) = evaluate(hash, r, s, x, y); console2.log("gasUsed ", gasUsed); assertEq(result, false); - assertLt(gasUsed, 1500); + assertLt(gasUsed, failFastGasBound); (x, y) = (1, 0); (result, gasUsed) = evaluate(hash, r, s, x, y); console2.log("gasUsed ", gasUsed); assertEq(result, false); - assertLt(gasUsed, 1500); + assertLt(gasUsed, failFastGasBound); (x, y) = (1, p); (result, gasUsed) = evaluate(hash, r, s, x, y); console2.log("gasUsed ", gasUsed); assertEq(result, false); - assertLt(gasUsed, 1500); + assertLt(gasUsed, failFastGasBound); (x, y) = (p, 1); (result, gasUsed) = evaluate(hash, r, s, x, y); console2.log("gasUsed ", gasUsed); assertEq(result, false); - assertLt(gasUsed, 1500); + assertLt(gasUsed, failFastGasBound); // p-1 is in-bounds but point is not on curve. (x, y) = (p - 1, 1); diff --git a/test/WebAuthn.t.sol b/test/WebAuthn.t.sol index 23912ca..270278c 100644 --- a/test/WebAuthn.t.sol +++ b/test/WebAuthn.t.sol @@ -21,8 +21,6 @@ contract WebAuthnTest is Test { 0x32e005a53ae49a96ac88c715243638dd5c985fbd463c727d8eefd05bee4e2570; uint256 s = 0x7a4fef4d0b11187f95f69eefbb428df8ac799bbd9305066b1e9c9fe9a5bcf8c4; - uint256 challengeLocation = 23; - uint256 responseTypeLocation = 1; function setUp() public { // Deploy P256 Verifier @@ -36,8 +34,6 @@ contract WebAuthnTest is Test { authenticatorData: authenticatorData, requireUserVerification: false, clientDataJSON: clientDataJSON, - challengeLocation: challengeLocation, - responseTypeLocation: responseTypeLocation, r: r, s: s, x: publicKey[0], @@ -46,21 +42,16 @@ contract WebAuthnTest is Test { assertTrue(ret); } - // Test failures in contains() function - function testContains() public { - bool ret; - uint256 customChallengeLocation = challengeLocation; - uint256 customResponseTypeLocation = responseTypeLocation; - - // Wrong challengeLocation - customChallengeLocation = 50; - ret = WebAuthn.verifySignature({ + // Test startsWith is implemented correctly + function testStartsWith() public { + // End too early + string + memory customClientDataJSON = '{"type":"webauthn.get","challenge":'; + bool ret = WebAuthn.verifySignature({ challenge: challenge, authenticatorData: authenticatorData, - requireUserVerification: false, - clientDataJSON: clientDataJSON, - challengeLocation: customChallengeLocation, - responseTypeLocation: customResponseTypeLocation, + requireUserVerification: true, + clientDataJSON: customClientDataJSON, r: r, s: s, x: publicKey[0], @@ -68,15 +59,13 @@ contract WebAuthnTest is Test { }); assertFalse(ret); - // challengeLocation out of bounds of string - customChallengeLocation = 100; + // missing { in beginning + customClientDataJSON = '"type":"webauthn.get","challenge":"dGVzdA","origin":"https://funny-froyo-3f9b75.netlify.app"}'; ret = WebAuthn.verifySignature({ challenge: challenge, authenticatorData: authenticatorData, - requireUserVerification: false, - clientDataJSON: clientDataJSON, - challengeLocation: customChallengeLocation, - responseTypeLocation: customResponseTypeLocation, + requireUserVerification: true, + clientDataJSON: customClientDataJSON, r: r, s: s, x: publicKey[0], @@ -84,16 +73,13 @@ contract WebAuthnTest is Test { }); assertFalse(ret); - // Wrong responseTypeLocation - customChallengeLocation = 23; - customResponseTypeLocation = 0; + // missing closing quote on challenge + customClientDataJSON = '{"type":"webauthn.get","challenge":"dGVzdA,"origin":"https://funny-froyo-3f9b75.netlify.app"}'; ret = WebAuthn.verifySignature({ challenge: challenge, authenticatorData: authenticatorData, - requireUserVerification: false, - clientDataJSON: clientDataJSON, - challengeLocation: customChallengeLocation, - responseTypeLocation: customResponseTypeLocation, + requireUserVerification: true, + clientDataJSON: customClientDataJSON, r: r, s: s, x: publicKey[0], @@ -114,8 +100,6 @@ contract WebAuthnTest is Test { authenticatorData: customAuthenticatorData, requireUserVerification: false, clientDataJSON: clientDataJSON, - challengeLocation: challengeLocation, - responseTypeLocation: responseTypeLocation, r: r, s: s, x: publicKey[0], @@ -130,8 +114,6 @@ contract WebAuthnTest is Test { authenticatorData: customAuthenticatorData, requireUserVerification: false, clientDataJSON: clientDataJSON, - challengeLocation: challengeLocation, - responseTypeLocation: responseTypeLocation, r: r, s: s, x: publicKey[0], @@ -147,8 +129,6 @@ contract WebAuthnTest is Test { authenticatorData: customAuthenticatorData, requireUserVerification: true, clientDataJSON: clientDataJSON, - challengeLocation: challengeLocation, - responseTypeLocation: responseTypeLocation, r: r, s: s, x: publicKey[0], @@ -165,8 +145,6 @@ contract WebAuthnTest is Test { authenticatorData: customAuthenticatorData, requireUserVerification: false, clientDataJSON: clientDataJSON, - challengeLocation: challengeLocation, - responseTypeLocation: responseTypeLocation, r: r, s: s, x: publicKey[0], @@ -183,8 +161,6 @@ contract WebAuthnTest is Test { authenticatorData: customAuthenticatorData, requireUserVerification: false, clientDataJSON: clientDataJSON, - challengeLocation: challengeLocation, - responseTypeLocation: responseTypeLocation, r: r, s: s, x: publicKey[0], @@ -201,8 +177,6 @@ contract WebAuthnTest is Test { authenticatorData: authenticatorData, requireUserVerification: true, clientDataJSON: clientDataJSON, - challengeLocation: challengeLocation, - responseTypeLocation: responseTypeLocation, r: r, s: s, x: publicKey[0],