Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@ All notable changes to this project are documented here. The format is based on
[Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project
adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Fixed
- **A trailing root-label dot on an allow-list ENTRY no longer silently blocks everything.**
`isAllowedHost` normalized the incoming hostname (trim, case-fold, strip the trailing dot) but
only trimmed and case-folded the configured entries, so
`LEXWARE_UPLOAD_ALLOWED_HOSTS=sharepoint.com.` matched nothing and blocked the very host it was
meant to allow. Both sides now go through the same normalization, so they cannot drift again. This
is the same failure mode as the leading dot fixed in 0.1.12, arriving from the other end of the
name; raised by the review bot on [#39] and not carried over at the time. A double trailing dot is
deliberately NOT folded — that is an empty DNS label, i.e. a malformed name rather than another
spelling of the same one.

## [0.1.13]

Robustness fixes surfaced by a full security audit and code-review pass of the server. No
Expand Down
24 changes: 19 additions & 5 deletions src/uploads/fetch-url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,31 @@ const DEFAULT_MAX_REDIRECTS = 3;
*/
export const DEFAULT_ALLOWED_HOSTS = ["sharepoint.com", "onedrive.live.com", "1drv.ms", "graph.microsoft.com"];

/**
* A host name reduced to its comparable form: trimmed, case-folded, and with the
* trailing root-label dot removed (`example.com.` and `example.com` are the same name;
* `new URL()` keeps the dot, so it reaches here).
*
* Used on BOTH sides of the comparison in {@link isAllowedHost}. A normalization applied
* to only one side is one that fails whenever the *other* side carries the odd spelling.
*
* Exactly one trailing dot, not `/\.+$/`: a second dot makes an empty DNS label, so
* `example.com..` is not another spelling of the same name and must not be folded into
* one — this step exists to equate equivalent names, not to repair malformed ones.
*/
function normalizeHostForMatch(value: string): string {
return value.trim().toLowerCase().replace(/\.$/, "");
}

/**
* True when `hostname` is exactly one of `allowed`, or a subdomain of one of them
* (matched on a dot boundary — "evilsharepoint.com" must NOT match "sharepoint.com").
* Case-insensitive; a trailing dot on `hostname` (a valid DNS root-label terminator) is
* stripped before comparison.
* Case-insensitive, and a trailing root-label dot on either side is ignored.
*/
export function isAllowedHost(hostname: string, allowed: string[]): boolean {
let host = hostname.trim().toLowerCase();
if (host.endsWith(".")) host = host.slice(0, -1);
const host = normalizeHostForMatch(hostname);
for (const entry of allowed) {
const suffix = entry.trim().toLowerCase();
const suffix = normalizeHostForMatch(entry);
if (!suffix) continue;
if (host === suffix || host.endsWith(`.${suffix}`)) return true;
}
Expand Down
25 changes: 25 additions & 0 deletions tests/uploads-fetch-url.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,31 @@ describe("isAllowedHost", () => {
expect(isAllowedHost("FOO.SharePoint.COM", DEFAULT_ALLOWED_HOSTS)).toBe(true);
expect(isAllowedHost("sharepoint.com.", DEFAULT_ALLOWED_HOSTS)).toBe(true);
});

it("ignores a trailing root-label dot on a CONFIGURED ENTRY too, not only on the hostname", () => {
// The normalization used to run on the hostname alone, so an operator who wrote the
// fully-qualified form in LEXWARE_UPLOAD_ALLOWED_HOSTS silently blocked the very host
// they meant to allow — the same failure mode as the leading dot fixed in 0.1.12,
// arriving from the other end of the name.
expect(isAllowedHost("contoso.sharepoint.com", ["sharepoint.com."])).toBe(true);
expect(isAllowedHost("sharepoint.com", ["sharepoint.com."])).toBe(true);
// Odd spelling on both sides at once still resolves to the same name.
expect(isAllowedHost("contoso.sharepoint.com.", ["sharepoint.com."])).toBe(true);
expect(isAllowedHost(" SharePoint.COM. ", [" .. "])).toBe(false);
});

it("still refuses a lookalike when the entry carries a trailing dot — the dot is not a wildcard", () => {
expect(isAllowedHost("evilsharepoint.com", ["sharepoint.com."])).toBe(false);
expect(isAllowedHost("sharepoint.com.evil.com", ["sharepoint.com."])).toBe(false);
});

it("does not fold a DOUBLE trailing dot into the same name — that is an empty label, not a spelling", () => {
// `example.com..` is malformed rather than equivalent, and this step exists to equate
// equivalent names, not to repair broken ones. Widening it would quietly grow what the
// allow-list accepts.
expect(isAllowedHost("sharepoint.com..", DEFAULT_ALLOWED_HOSTS)).toBe(false);
expect(isAllowedHost("sharepoint.com", ["sharepoint.com.."])).toBe(false);
});
});

describe("fetchRemoteFile", () => {
Expand Down