Ensure all signatures returned by beforeSign hooks are valid

src/session.ts

@@ -5,6 +5,7 @@ import {
     NameType,
     PermissionLevel,
     PermissionLevelType,
+    Serializer,
     Signature,
     SignedTransaction,
 } from '@greymass/eosio'
@@ -77,6 +78,7 @@ export interface SessionOptions {
     permissionLevel?: PermissionLevelType | string
     transactPlugins?: AbstractTransactPlugin[]
     transactPluginsOptions?: TransactPluginsOptions
+    validatePluginSignatures?: boolean
     walletPlugin: WalletPlugin
 }
 
@@ -90,6 +92,7 @@ export class Session {
     readonly permissionLevel: PermissionLevel
     readonly transactPlugins: TransactPlugin[]
     readonly transactPluginsOptions: TransactPluginsOptions = {}
+    readonly validatePluginSignatures: boolean = true
     readonly wallet: WalletPlugin
 
     constructor(options: SessionOptions) {
@@ -125,6 +128,9 @@ export class Session {
                 'Either a permissionLevel or actor/permission must be provided when creating a new Session.'
             )
         }
+        if (options.validatePluginSignatures !== undefined) {
+            this.validatePluginSignatures = options.validatePluginSignatures
+        }
         this.wallet = options.walletPlugin
     }
 
@@ -292,6 +298,7 @@ export class Session {
         // Create response template to this transact call
         const result: TransactResult = {
             chain: this.chain,
+            keys: [],
             request,
             resolved: undefined,
             revisions: new TransactRevisions(request),
@@ -314,6 +321,12 @@ export class Session {
         const willBroadcast =
             options && typeof options.broadcast !== 'undefined' ? options.broadcast : this.broadcast
 
+        // Whether all signatures generated
+        const willValidatePluginSignatures =
+            options && typeof options.validatePluginSignatures !== 'undefined'
+                ? options.validatePluginSignatures
+                : this.validatePluginSignatures
+
         // Run the `beforeSign` hooks
         for (const hook of context.hooks.beforeSign) {
             // Get the response of the hook by passing a clonied request.
@@ -326,12 +339,29 @@ export class Session {
             if (allowModify) {
                 request = await this.updateRequest(request, response.request, abiCache)
             }
-            // If signatures were returned, append them
-            if (response.signatures) {
+
+            // If signatures were returned, record them in the response.
+            if (response.signatures?.length) {
+                // Merge new signatures alongside existing signatures into the TransactResult.
                 result.signatures = [...result.signatures, ...response.signatures]
+
+                // Recover the keys used to generate the signatures at the time of the request.
+                const recoveredKeys = response.signatures.map((signature) => {
+                    const requestTransaction = request.getRawTransaction()
+                    const requestDigest = requestTransaction.signingDigest(this.chain.id)
+                    return signature.recoverDigest(requestDigest)
+                })
+
+                // Merge newly discovered keys into the TransactResult.
+                result.keys = [...result.keys, ...recoveredKeys]
             }
         }
 
+        // Validate all the signatures returned by the plugins against the current request
+        if (willValidatePluginSignatures) {
+            this.validateBeforeSignSignatures(context, request, result)
+        }
+
         // Resolve the SigningRequest and assign it to the TransactResult
         result.request = request
         result.resolved = await context.resolve(request, expireSeconds)
@@ -362,5 +392,26 @@ export class Session {
 
         return result
     }
+    validateBeforeSignSignatures(
+        context: TransactContext,
+        request: SigningRequest,
+        result: TransactResult
+    ): void {
+        const requestTransaction = request.getRawTransaction()
+        const requestDigest = requestTransaction.signingDigest(this.chain.id)
+        const publicKeys = Serializer.objectify(result.keys)
+        result.signatures.forEach((signature) => {
+            const recoveredKey = signature.recoverDigest(requestDigest)
+            const verified = signature.verifyDigest(requestDigest, recoveredKey)
+            if (!verified || !publicKeys.includes(String(recoveredKey))) {
+                throw new Error(
+                    `A signature (${signature}) provided by a beforeSign hook using ` +
+                        `a key (${recoveredKey}) has been invalidated, likely due to the transaction ` +
+                        `being modified by another hook after the signature was created. To disable ` +
+                        `this error, set validatePluginSignatures equal to false.`
+                )
+            }
+        })
+    }
 }
 export {AbstractTransactPlugin}

src/transact.ts

@@ -5,6 +5,7 @@ import {
     Checksum256Type,
     Name,
     PermissionLevel,
+    PublicKey,
     Serializer,
     Signature,
 } from '@greymass/eosio'
@@ -226,6 +227,8 @@ export class TransactRevisions {
 export interface TransactResult {
     /** The chain that was used. */
     chain: ChainDefinition
+    /** The public keys used to sign. */
+    keys: PublicKey[]
     /** The SigningRequest representation of the transaction. */
     request: SigningRequest
     /** The ResolvedSigningRequest of the transaction */

test/tests/transact.ts

@@ -1,7 +1,18 @@
 import {assert} from 'chai'
 import zlib from 'pako'
 
-import {Action, Name, PermissionLevel, Serializer, Signature, TimePointSec} from '@greymass/eosio'
+import {
+    Action,
+    Name,
+    PermissionLevel,
+    PrivateKey,
+    Serializer,
+    Signature,
+    SignatureType,
+    Struct,
+    TimePointSec,
+    Transaction,
+} from '@greymass/eosio'
 import {ResolvedSigningRequest, SigningRequest} from 'eosio-signing-request'
 
 import SessionKit, {
@@ -321,6 +332,128 @@ suite('transact', function () {
                 )
             })
         })
+        suite('validatePluginSignatures', function () {
+            test('default: true (check and throw after mutations invalidate signatures)', async function () {
+                const {action, session} = await mockData()
+                const randomKeyCosignerHook = async (
+                    request: SigningRequest,
+                    context: TransactContext
+                ) => {
+                    const randomName = Name.from('')
+                    const randomKey = PrivateKey.generate('K1')
+                    class noop extends Struct {
+                        static abiName = 'noop'
+                        static abiFields = []
+                    }
+                    const newAction = Action.from({
+                        account: 'greymassnoop',
+                        name: 'noop',
+                        authorization: [
+                            {
+                                actor: randomName,
+                                permission: 'cosign',
+                            },
+                        ],
+                        data: noop.from({}),
+                    })
+                    const info = await context.client.v1.chain.get_info()
+                    const header = info.getTransactionHeader()
+                    const newTransaction = Transaction.from({
+                        ...header,
+                        actions: [newAction, ...request.getRawActions()],
+                    })
+                    const newRequest = await SigningRequest.create(
+                        {transaction: newTransaction, chainId: session.chain.id},
+                        context.esrOptions
+                    )
+                    const digest = newTransaction.signingDigest(info.chain_id)
+                    const signature: SignatureType = randomKey.signDigest(digest)
+                    return {
+                        request: newRequest,
+                        signatures: [signature],
+                    }
+                }
+                const debugPlugin = {
+                    register(context) {
+                        context.addHook(TransactHookTypes.beforeSign, randomKeyCosignerHook)
+                        context.addHook(TransactHookTypes.beforeSign, randomKeyCosignerHook)
+                    },
+                }
+                let error
+                await session
+                    .transact(
+                        {action},
+                        {
+                            broadcast: false,
+                            transactPlugins: [debugPlugin],
+                        }
+                    )
+                    .catch((e) => {
+                        error = e
+                    })
+                assert.instanceOf(
+                    error,
+                    Error,
+                    'Expected an Error to be thrown by validatePluginSignatures.'
+                )
+            })
+            test('override: false', async function () {
+                const {action, session} = await mockData()
+                const randomKeyCosignerHook = async (
+                    request: SigningRequest,
+                    context: TransactContext
+                ) => {
+                    const randomName = Name.from('')
+                    const randomKey = PrivateKey.generate('K1')
+                    class noop extends Struct {
+                        static abiName = 'noop'
+                        static abiFields = []
+                    }
+                    const newAction = Action.from({
+                        account: 'greymassnoop',
+                        name: 'noop',
+                        authorization: [
+                            {
+                                actor: randomName,
+                                permission: 'cosign',
+                            },
+                        ],
+                        data: noop.from({}),
+                    })
+                    const info = await context.client.v1.chain.get_info()
+                    const header = info.getTransactionHeader()
+                    const newTransaction = Transaction.from({
+                        ...header,
+                        actions: [newAction, ...request.getRawActions()],
+                    })
+                    const newRequest = await SigningRequest.create(
+                        {transaction: newTransaction, chainId: session.chain.id},
+                        context.esrOptions
+                    )
+                    const digest = newTransaction.signingDigest(info.chain_id)
+                    const signature: SignatureType = randomKey.signDigest(digest)
+                    return {
+                        request: newRequest,
+                        signatures: [signature],
+                    }
+                }
+                const debugPlugin = {
+                    register(context) {
+                        context.addHook(TransactHookTypes.beforeSign, randomKeyCosignerHook)
+                        context.addHook(TransactHookTypes.beforeSign, randomKeyCosignerHook)
+                    },
+                }
+                const result = await session.transact(
+                    {action},
+                    {
+                        broadcast: false,
+                        transactPlugins: [debugPlugin],
+                        validatePluginSignatures: false,
+                    }
+                )
+                assetValidTransactResponse(result)
+            })
+        })
         suite('transactPlugins', function () {
             test('inherit', async function () {
                 const {action} = await mockData()