CVE-2022-35917: Always-Incorrect Control Flow Implementation (CWE-670)
value: Solana Pay is a protocol and set of reference implementations that enable developers to incorporate decentralized payments into their apps and services. When a Solana Pay transaction is located using a reference key, it may be checked to represent a transfer of the desired amount to the recipient, using the supplied `validateTransfer` function. An edge case regarding this mechanism could cause the validation logic to validate multiple transfers. This issue has been patched as of version `0.2.1`. Users of the Solana Pay SDK should upgrade to it. There are no known workarounds for this issue.
CWE description: The code contains a control flow path that does not reflect the algorithm that the path is intended to implement, leading to incorrect behavior any time this path is navigated.
CVSS v3 base score: 5.3
Published: 2022-08-01T22:15Z
Repository: https://github.com/solana-labs/solana-pay
Language: TypeScript
Files changed: core/src/validateTransfer.ts
Commit date: 2022-07-28 12:15:33 -0500
Commit message: finish transfer validation implementation

Diff: diff --git a/core/src/validateTransfer.ts b/core/src/validateTransfer.ts
index 9af9d11..f448669 100644
--- a/core/src/validateTransfer.ts
+++ b/core/src/validateTransfer.ts
@@ -1,17 +1,25 @@
-import { getAssociatedTokenAddress } from '@solana/spl-token';
+import {
+    decodeInstruction,
+    getAssociatedTokenAddress,
+    isTransferCheckedInstruction,
+    isTransferInstruction,
+} from '@solana/spl-token';
 import {
     ConfirmedTransactionMeta,
     Connection,
     Finality,
     LAMPORTS_PER_SOL,
     Message,
+    SystemInstruction,
+    Transaction,
     TransactionResponse,
     TransactionSignature,
 } from '@solana/web3.js';
 import BigNumber from 'bignumber.js';
-import { Amount, Memo, Recipient, References, SPLToken } from './types';
+import { MEMO_PROGRAM_ID } from './constants';
+import { Amount, Memo, Recipient, Reference, References, SPLToken } from './types';
 
 /**
  * Thrown when a transaction doesn't contain a valid Solana Pay transfer.
  */
 export class ValidateTransferError extends Error {
@@ -56,37 +64,53 @@ export async function validateTransfer(
     const message = response.transaction.message;
     const meta = response.meta;
     if (!meta) throw new ValidateTransferError('missing meta');
     if (meta.err) throw meta.err;
 
-    const [preAmount, postAmount] = splToken
-        ? await validateSPLTokenTransfer(message, meta, recipient, splToken)
-        : await validateSystemTransfer(message, meta, recipient);
+    if (reference && !Array.isArray(reference)) {
+        reference = [reference];
+    }
 
+    const [preAmount, postAmount] = splToken
+        ? await validateSPLTokenTransfer(message, meta, recipient, splToken, reference)
+        : await validateSystemTransfer(message, meta, recipient, reference);
     if (postAmount.minus(preAmount).lt(amount)) throw new ValidateTransferError('amount not transferred');
 
-    if (reference) {
-        if (!Array.isArray(reference)) {
-            reference = [reference];
-        }
-
-        for (const pubkey of reference) {
-            if (!message.accountKeys.some((accountKey) => accountKey.equals(pubkey)))
-                throw new ValidateTransferError('reference not found');
-        }
+    if (memo) {
+        // Check that the second instruction is a memo instruction with the expected memo.
+        const transaction = Transaction.populate(message);
+        const instruction = transaction.instructions[1];
+        if (!instruction) throw new ValidateTransferError('missing memo instruction');
+        if (!instruction.programId.equals(MEMO_PROGRAM_ID)) throw new ValidateTransferError('invalid memo program');
+        if (!instruction.data.equals(Buffer.from(memo, 'utf8'))) throw new ValidateTransferError('invalid memo');
     }
 
-    // FIXME: add memo check
-
     return response;
 }
 
 async function validateSystemTransfer(
     message: Message,
     meta: ConfirmedTransactionMeta,
-    recipient: Recipient
+    recipient: Recipient,
+    references?: Reference[]
 ): Promise<[BigNumber, BigNumber]> {
+    if (references) {
+        // Check that the first instruction is a system transfer instruction.
+        const transaction = Transaction.populate(message);
+        const instruction = transaction.instructions[0];
+        SystemInstruction.decodeTransfer(instruction);
+
+        // Check that the expected reference keys exactly match the extra keys provided to the instruction.
+        const [_from, _to, ...extraKeys] = instruction.keys;
+        const length = extraKeys.length;
+        if (length !== references.length) throw new ValidateTransferError('invalid references');
+
+        for (let i = 0; i < length; i++) {
+            if (!extraKeys[i].pubkey.equals(references[i])) throw new ValidateTransferError(`invalid reference ${i}`);
+        }
+    }
+
     const accountIndex = message.accountKeys.findIndex((pubkey) => pubkey.equals(recipient));
     if (accountIndex === -1) throw new ValidateTransferError('recipient not found');
 
     return [
         new BigNumber(meta.preBalances[accountIndex] || 0).div(LAMPORTS_PER_SOL),
@@ -96,12 +120,30 @@ async function validateSystemTransfer(
 
 async function validateSPLTokenTransfer(
     message: Message,
     meta: ConfirmedTransactionMeta,
     recipient: Recipient,
-    splToken: SPLToken
+    splToken: SPLToken,
+    references?: Reference[]
 ): Promise<[BigNumber, BigNumber]> {
+    if (references) {
+        // Check that the first instruction is an SPL token transfer instruction.
+        const transaction = Transaction.populate(message);
+        const instruction = decodeInstruction(transaction.instructions[0]);
+        if (!isTransferCheckedInstruction(instruction) && !isTransferInstruction(instruction))
+            throw new ValidateTransferError('invalid transfer');
+
+        // Check that the expected reference keys exactly match the extra keys provided to the instruction.
+        const extraKeys = instruction.keys.multiSigners;
+        const length = extraKeys.length;
+        if (length !== references.length) throw new ValidateTransferError('invalid references');
+
+        for (let i = 0; i < length; i++) {
+            if (!extraKeys[i].pubkey.equals(references[i])) throw new ValidateTransferError(`invalid reference ${i}`);
+        }
+    }
+
     const recipientATA = await getAssociatedTokenAddress(splToken, recipient);
     const accountIndex = message.accountKeys.findIndex((pubkey) => pubkey.equals(recipientATA));
     if (accountIndex === -1) throw new ValidateTransferError('recipient not found');
 
     const preBalance = meta.preTokenBalances?.find((x) => x.accountIndex === accountIndex);

Vulnerable code: import { getAssociatedTokenAddress } from '@solana/spl-token';
import { Amount, Memo, Recipient, References, SPLToken } from './types';
    const [preAmount, postAmount] = splToken
        ? await validateSPLTokenTransfer(message, meta, recipient, splToken)
        : await validateSystemTransfer(message, meta, recipient);
    if (reference) {
        if (!Array.isArray(reference)) {
            reference = [reference];
        }

        for (const pubkey of reference) {
            if (!message.accountKeys.some((accountKey) => accountKey.equals(pubkey)))
                throw new ValidateTransferError('reference not found');
        }
    // FIXME: add memo check

    recipient: Recipient
    splToken: SPLToken
Fixed code: import {
    decodeInstruction,
    getAssociatedTokenAddress,
    isTransferCheckedInstruction,
    isTransferInstruction,
} from '@solana/spl-token';
    SystemInstruction,
    Transaction,
import { MEMO_PROGRAM_ID } from './constants';
import { Amount, Memo, Recipient, Reference, References, SPLToken } from './types';
    if (reference && !Array.isArray(reference)) {
        reference = [reference];
    }
    const [preAmount, postAmount] = splToken
        ? await validateSPLTokenTransfer(message, meta, recipient, splToken, reference)
        : await validateSystemTransfer(message, meta, recipient, reference);
    if (memo) {
        // Check that the second instruction is a memo instruction with the expected memo.
        const transaction = Transaction.populate(message);
        const instruction = transaction.instructions[1];
        if (!instruction) throw new ValidateTransferError('missing memo instruction');
        if (!instruction.programId.equals(MEMO_PROGRAM_ID)) throw new ValidateTransferError('invalid memo program');
        if (!instruction.data.equals(Buffer.from(memo, 'utf8'))) throw new ValidateTransferError('invalid memo');
    recipient: Recipient,
    references?: Reference[]
    if (references) {
        // Check that the first instruction is a system transfer instruction.
        const transaction = Transaction.populate(message);
        const instruction = transaction.instructions[0];
        SystemInstruction.decodeTransfer(instruction);

        // Check that the expected reference keys exactly match the extra keys provided to the instruction.
        const [_from, _to, ...extraKeys] = instruction.keys;
        const length = extraKeys.length;
        if (length !== references.length) throw new ValidateTransferError('invalid references');

        for (let i = 0; i < length; i++) {
            if (!extraKeys[i].pubkey.equals(references[i])) throw new ValidateTransferError(`invalid reference ${i}`);
        }
    }

    splToken: SPLToken,
    references?: Reference[]
    if (references) {
        // Check that the first instruction is an SPL token transfer instruction.
        const transaction = Transaction.populate(message);
        const instruction = decodeInstruction(transaction.instructions[0]);
        if (!isTransferCheckedInstruction(instruction) && !isTransferInstruction(instruction))
            throw new ValidateTransferError('invalid transfer');

        // Check that the expected reference keys exactly match the extra keys provided to the instruction.
        const extraKeys = instruction.keys.multiSigners;
        const length = extraKeys.length;
        if (length !== references.length) throw new ValidateTransferError('invalid references');

        for (let i = 0; i < length; i++) {
            if (!extraKeys[i].pubkey.equals(references[i])) throw new ValidateTransferError(`invalid reference ${i}`);
        }
    }