Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ A breaking change will get clearly marked in this log.
* Fixed bigint-to-U32/I32 conversion in `Spec` using `Number(val)` instead of `val as number` (a no-op for bigints) ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
* Fixed missing template literal `$` in two `Spec` error messages that were not interpolated ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
* WASM custom section parser: when a section was skipped (invalid name length), the offset was not advanced, causing an infinite loop or incorrect parsing of subsequent sections ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
* `FederationServer.resolve` now validates domains per RFC 1035, rejecting malformed domains. Port numbers are also accepted in the domain ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
* `FederationServer.createForDomain` now validates domains per RFC 1035, rejecting malformed domains. Port numbers are also accepted in the domain ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
Comment thread
Ryang-21 marked this conversation as resolved.
Outdated
* `FederationServer` URL mutation: `resolveAddress`, `resolveAccountId`, and `resolveTransactionId` mutated the shared `serverURL` by appending query params on each call. Fixed by cloning the URL before modifying ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
* `CallBuilder.stream()` URL mutation: `stream()` mutated the shared `this.url` by adding query params, corrupting the builder for subsequent calls. Fixed by cloning the URL ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
* `AssembledTransaction` restore path: when `buildWithOp` was used and automatic state restoration was needed, the rebuild incorrectly reconstructed the operation via `contract.call()` instead of reusing the original operation ([#1373](https://github.com/stellar/js-stellar-sdk/pull/1373)).
Expand Down
37 changes: 26 additions & 11 deletions src/federation/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,6 @@ export class FederationServer {
return Promise.reject(new Error("Invalid Stellar address"));
}

// Validate domain per RFC 1035 (as required by SEP-0002): each dot-separated
// label must start with a letter, end with a letter or digit, and contain only
// letters, digits, or hyphens.
if (
!/^(?:[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?\.)*[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?(?::\d+)?$/.test(
domain,
)
) {
return Promise.reject(new Error("Invalid domain in Stellar address"));
}
const federationServer = await FederationServer.createForDomain(
domain,
opts,
Expand Down Expand Up @@ -135,6 +125,20 @@ export class FederationServer {
domain: string,
opts: Api.Options = {},
): Promise<FederationServer> {
// Validate domain per RFC 1035 (as required by SEP-0002): each dot-separated
// label must start with a letter, end with a letter or digit, and contain only
// letters, digits, or hyphens.
if (
!/^(?:[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?\.)*[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?(?::\d+)?$/.test(
domain,
Comment thread
Ryang-21 marked this conversation as resolved.
Outdated
)
) {
return Promise.reject(
new Error(
"The provided domain is invalid. Ensure that the domain adheres to RFC 1035",
),
);
}
const tomlObject = await Resolver.resolve(domain, opts);
if (!tomlObject.FEDERATION_SERVER) {
return Promise.reject(
Expand All @@ -152,7 +156,18 @@ export class FederationServer {
// TODO `domain` regexp
Comment thread
Ryang-21 marked this conversation as resolved.
Outdated
this.serverURL = URI(serverURL);
this.domain = domain;

// Validate domain per RFC 1035 (as required by SEP-0002): each dot-separated
// label must start with a letter, end with a letter or digit, and contain only
// letters, digits, or hyphens.
if (
!/^(?:[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?\.)*[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?(?::\d+)?$/.test(
domain,
)
) {
Comment thread
Ryang-21 marked this conversation as resolved.
Outdated
throw new Error(
"The provided domain is invalid. Ensure that the domain adheres to RFC 1035",
);
Comment thread
Ryang-21 marked this conversation as resolved.
Outdated
}
const allowHttp =
typeof opts.allowHttp === "undefined"
? Config.isAllowHttp()
Expand Down
13 changes: 13 additions & 0 deletions test/unit/federation_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ describe("federation-server.js tests", () => {
).toThrow(/Cannot connect to insecure federation server/);
});

it("throws error for invalid domain", () => {
expect(
() => new Server("https://acme.com:1337/federation", "-stellar.org"),
).toThrow(/The provided domain is invalid/);
});

it("allow insecure server when opts.allowHttp flag is set", () => {
expect(
() =>
Expand Down Expand Up @@ -170,6 +176,13 @@ FEDERATION_SERVER="https://api.stellar.org/federation"
expect(federationServer["domain"]).toEqual("acme.com");
});

it("fails for invalid domain before requesting stellar.toml", async () => {
await expect(Server.createForDomain("-acme.com")).rejects.toThrow(
/The provided domain is invalid/,
);
expect(mockHttpClient).not.toHaveBeenCalled();
});

it("fails when stellar.toml does not contain federation server info", async () => {
mockHttpClient.mockImplementation((url: string) => {
if (url.includes("https://acme.com/.well-known/stellar.toml")) {
Expand Down
Loading