Check staticcall Result From SHA-256 Precompile

This PR changes the `_sha256` implementation to check the result from
the static call. There is a very subtle bug with not checking, where,
for very large inputs, you would be able to get the precompile to revert
but have the function finish executing successfully (and use whatever is
in the scratch space as the digest).

Note that **we do not check the length of the `returndata`**. This is
intentional and the same thing that the Solidity compiler does for the
builtin `sha256` function.

modules/passkey/contracts/libraries/WebAuthn.sol

@@ -339,11 +339,17 @@ library WebAuthn {
     function _sha256(bytes memory input) private view returns (bytes32 digest) {
         // solhint-disable-next-line no-inline-assembly
         assembly ("memory-safe") {
-            // The SHA-256 precompile is at address 0x0002. Note that we don't check the whether or
-            // not the precompile reverted or if the return data size is 32 bytes, which is a
-            // reasonable assumption for the precompile, as it is specified to always return the
-            // SHA-256 of its input bytes.
-            pop(staticcall(gas(), 0x0002, add(input, 0x20), mload(input), 0, 32))
+            // The SHA-256 precompile is at address 0x0002. Note that checking the result of the
+            // call is important to prevent callers from setting gas to specific values that would
+            // cause the call to the precompile to revert, but the function to continue executing
+            // and have digest be whatever was in the scratch space at the time of the call.
+            // However, we do not check if the return data size is 32 bytes, which is a reasonable
+            // assumption for the precompile, as it is specified to always return the SHA-256 of
+            // its input bytes on success. Note that this is similar to the code generated by the
+            // Solidity compiler for the `sha256` built-in.
+            if iszero(staticcall(gas(), 0x0002, add(input, 0x20), mload(input), 0, 32)) {
+                revert(0, 0)
+            }
             digest := mload(0)
         }
     }

modules/passkey/test/libraries/WebAuthn.spec.ts

@@ -131,7 +131,7 @@ describe('WebAuthn Library', () => {
     })
   })
 
-  describe('signingMessage', function () {
+  describe('encodeSigningMessage', function () {
     it('Should correctly compute a signing message', async () => {
       const { webAuthnLib } = await setupTests()
 
@@ -150,6 +150,19 @@ describe('WebAuthn Library', () => {
 
       expect(await webAuthnLib.encodeSigningMessage(challenge, authenticatorData, `"origin":"http://safe.global"`)).to.equal(message)
     })
+
+    it('Should revert if SHA-256 precompile reverts', async () => {
+      const { webAuthnLib } = await setupTests()
+
+      // This test is a bit tricky - the SHA-256 precompile can be made to revert by calling it
+      // with insufficient gas. Here we check that the revert is propagated by the
+      // `encodeSigningMessage` function. If the revert were not propagated, since the input is
+      // large enough, the function would be able to finish executing and return bogus data. Finding
+      // a large enough client data and exact gas limits to make this happen is a bit annoying, so
+      // lets hope for no gas schedule changes :fingers_crossed:.
+      const longClientDataFields = `"long":"${'a'.repeat(100000)}"`
+      await expect(webAuthnLib.encodeSigningMessage(ethers.ZeroHash, '0x', longClientDataFields, { gasLimit: 1701001 })).to.be.reverted
+    })
   })
 
   describe('verifySignature', function () {