Skip to content

Commit

Permalink
Merge pull request from GHSA-6cgh-hjpw-q3gq
Browse files Browse the repository at this point in the history
* Add test to ensure readChallengeTx verifies the server signed the challenge

* Fix readChallengeTx not verifying server signature

* Update changelog

* Update version
  • Loading branch information
leighmcculloch authored Jul 2, 2021
1 parent ac46a8d commit 6f0bb88
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 1 deletion.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

This comment has been minimized.

Copy link
@dope2684

dope2684 Jul 4, 2021

I accept


### 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)

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
6 changes: 6 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}

Expand Down
35 changes: 35 additions & 0 deletions test/unit/utils_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down

0 comments on commit 6f0bb88

Please sign in to comment.