From 6f0bb889c2d10b431ddd5f4a1bcdd519c80430b3 Mon Sep 17 00:00:00 2001
From: Leigh McCulloch <leigh@stellar.org>
Date: Fri, 2 Jul 2021 08:46:09 -0700
Subject: [PATCH] Merge pull request from GHSA-6cgh-hjpw-q3gq

* Add test to ensure readChallengeTx verifies the server signed the challenge

* Fix readChallengeTx not verifying server signature

* Update changelog

* Update version
---
 CHANGELOG.md            |  5 +++++
 package.json            |  2 +-
 src/utils.ts            |  6 ++++++
 test/unit/utils_test.js | 35 +++++++++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0a391462f..bfb1471ee 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,11 @@ A breaking change will get clearly marked in this log.
 
 ## Unreleased
 
+## [v8.2.3](https://github.com/stellar/js-stellar-sdk/compare/v8.2.2...v8.2.3)
+
+### Fix
+- Fix server signature verification in `Utils.readChallengeTx`. The function was
+not verifying the server account had signed the challenge transaction.
 
 ## [v8.2.2](https://github.com/stellar/js-stellar-sdk/compare/v8.2.1...v8.2.2)
 
diff --git a/package.json b/package.json
index 05d12e4e2..06180d2b4 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "stellar-sdk",
-  "version": "8.2.2",
+  "version": "8.2.3",
   "description": "stellar-sdk is a library for working with the Stellar Horizon server.",
   "main": "./lib/index.js",
   "types": "./lib/index.d.ts",
diff --git a/src/utils.ts b/src/utils.ts
index a31b18056..cd206c726 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -265,6 +265,12 @@ export namespace Utils {
       }
     }
 
+    if (!verifyTxSignedBy(transaction, serverAccountID)) {
+      throw new InvalidSep10ChallengeError(
+        `Transaction not signed by server: '${serverAccountID}'`,
+      );
+    }
+
     return { tx: transaction, clientAccountID, matchedHomeDomain };
   }
 
diff --git a/test/unit/utils_test.js b/test/unit/utils_test.js
index 35e6354d0..343e1c9f1 100644
--- a/test/unit/utils_test.js
+++ b/test/unit/utils_test.js
@@ -216,6 +216,41 @@ describe('Utils', function() {
       });
     });
 
+    it("throws an error if the server hasn't signed the transaction", function () {
+      let serverKP = StellarSdk.Keypair.random();
+      let clientKP = StellarSdk.Keypair.random();
+
+      const transaction = new StellarSdk.TransactionBuilder(
+        new StellarSdk.Account(serverKP.publicKey(), "-1"),
+        { fee: 100, networkPassphrase: StellarSdk.Networks.TESTNET },
+      )
+        .addOperation(StellarSdk.Operation.manageData({
+          source: clientKP.publicKey(),
+          name: "SDF-test auth",
+          value: randomBytes(48).toString("base64"),
+        }))
+        .setTimeout(30)
+        .build();
+
+      const challenge = transaction
+        .toEnvelope()
+        .toXDR("base64")
+        .toString();
+
+      expect(() =>
+        StellarSdk.Utils.readChallengeTx(
+          challenge,
+          serverKP.publicKey(),
+          StellarSdk.Networks.TESTNET,
+          "SDF-test",
+          "testanchor.stellar.org"
+        ),
+      ).to.throw(
+        StellarSdk.InvalidSep10ChallengeError,
+        "Transaction not signed by server: '" + serverKP.publicKey() + "'",
+      );
+    });
+
     it("throws an error if transaction sequenceNumber is different to zero", function() {
       let keypair = StellarSdk.Keypair.random();