Add Fortify THREAT_MODEL to new d3 repo Bug: b/548453648 Change-Id: I029ac3a1f0e91cd526fb52f41a1b160338b53a8f Reviewed-on: https://gnocchi-internal-review.git.corp.google.com/c/third_party/d3/+/318824 Reviewed-by: Trevor Ryland <tryland@google.com>
diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md new file mode 100644 index 0000000..b84f401 --- /dev/null +++ b/THREAT_MODEL.md
@@ -0,0 +1,365 @@ +# Security Threat Model: d3 (AppSheet Fork) + +## Asset Definition & Scope + +- **Component:** D3.js — a JavaScript library for manipulating documents based + on data. Upstream is `d3/d3`; per `METADATA` the fork's base is upstream + **v3.4.2**, released **February 2014**. D3 v3 is end-of-life; upstream is now + at v7 and no security fixes are backported to the v3 line. +- **Repository:** Git-on-Borg + (`https://gnocchi-internal.googlesource.com/third_party/d3`, branch `main`). +- **History:** Two commits — `6776099` "Initial pristine commit of AppSheet OSS + D3 dependency" (parentless import of upstream tag `v3.4.2`, + `4637ee5a49ec9f2edea217869a0b4f43b3617912`) and `b081036` "Remove the jsdom + dependency and code we do not use". +- **Licence:** BSD-3-Clause (`METADATA` `license_type: NOTICE`). +- **Scope:** `d3.min.js` (146.8 KB — **the artefact that ships**), `d3.js` + (326.2 KB, the readable build), and `src/` (270 authored source files from + which both are generated). + +> [!IMPORTANT] +> **This fork is the confirmed build source of live, Internet-facing production +> JavaScript.** `d3.min.js` in this repository is **byte-identical** (MD5 +> `33a4640a0fc7e2a0ab47ec00b506f4c7`, verified) to the file checked into the +> AppSheet web frontend at `jeenee/Nirvana/Scripts/app/plugins/d3.min.js`, which +> is served to authenticated AppSheet users. Findings here are directly +> exploitable findings in AppSheet. + +**Version identity is intact — unlike the sibling nvd3 fork.** `package.json` +keeps `"version": "3.4.2"`, and `bin/start` stamps that value into +`src/start.js` as `var d3 = {version: "3.4.2"}`, so the runtime global reports +`d3.version === "3.4.2"`. Version-keyed SCA and advisory matching **will** work +against this asset. (The nvd3 fork deliberately detaches its version string; +that caveat does not apply here.) + +**Migration in flight.** AppSheet is moving from the vendored copy to consuming +this repository directly as an npm dependency +(`git+https://appsheet-third-party.googlesource.com/d3#<sha>`), loaded through a +webpack shim (`scripts/_shared/d3Init.ts`) that exposes `window.d3`. Before and +after that migration this repository is the source of truth for the shipped +bytes; only the delivery path changes. + +## Consumption in AppSheet + +`d3.min.js` is loaded by four live legacy Razor views, all with a CSP nonce: + +| View | Line | Surface | +|---|---|---| +| `V3/MainServer/Views/template/commoneditor/appStatsPartial.cshtml` | 101 | App usage statistics | +| `V3/MainServer/Views/template/_showTablePartial.cshtml` | 92 | App editor — table/column detail | +| `V3/MainServer/Views/Shared/_perfDetailsPartial.cshtml` | 57 | Performance / expression trace detail | +| `V3/MainServer/Views/Shared/_logDetailsPartial.cshtml` | 77 | Audit-log detail | + +These are **the same four views that load nvd3**. D3 is present primarily as +nvd3's prerequisite: nvd3 requires a global `d3` (v3) and its threat model +explicitly scopes D3 out — *"D3.js itself. The library requires a global `d3` +(v3), supplied separately by AppSheet; it is not vendored in this repository."* +**This document closes that gap.** The two assets must be read together. + +**Direct D3 API use by AppSheet code is small and well-bounded:** + +| Call | Location | +|---|---| +| `d3.select('#' + $graph.attr('id'))` | `Nirvana/Scripts/sp-theme/manageGraphs.js:117`, `:343` | +| `d3.select($selector[0])` | `_showTablePartial.cshtml:235`, `_perfDetailsPartial.cshtml:387` | +| `d3.select('#LogVirtualColumnPerformanceGraph .graphDisplay')` | `_perfDisplayPartial.cshtml:172` | +| `d3.format(',.0f')` | `manageGraphs.js:139`, `:304` | +| `d3.time.days(...)` / `d3.time.scale` | `manageGraphs.js:182`, `:324` | + +**Nothing in AppSheet calls `d3.csv`, `d3.tsv`, `d3.json`, `d3.html`, `d3.xhr`, +`d3.text`, `d3.xml`, or `selection.html()` directly.** The highest-severity +primitives catalogued below are therefore **shipped and exported but not +presently exercised by AppSheet's own code** — latent rather than live. They +remain one line of caller code away, and `.html()` *is* reached indirectly +through nvd3 (see Prompt 2). + +## The AppSheet Customization + +Commit `b081036`. **Deletions and metadata only — no behavioural patches.** + +- Deleted `index.js`, the jsdom-based server-side shim. Because upstream + declared `jsdom` as a *production* dependency, npm promoted its entire + subtree into AppSheet's production closure: **540 packages with it, 498 + without — 42 packages attributable solely to the `d3 → jsdom` edge**, along + with the five manual licence-review issues they generated. Removing it is the + stated reason the fork exists, per `METADATA`. +- Deleted `test/` (209 files, the only other jsdom consumer), `bin/bower`, + `bin/component`, `bower.json`, `component.json`, `composer.json`, + `.travis.yml`, `CONTRIBUTING.md`. +- `package.json`: `"main"` `index.js` → `d3.js`; removed `dependencies.jsdom`; + removed `vows` / `seedrandom` devDependencies and `scripts.test`; replaced + the legacy `"licenses": [{"type": "BSD", …}]` array with the SPDX + `"license": "BSD-3-Clause"`. +- `Makefile`: dropped `bower.json` / `component.json` from `GENERATED_FILES`, + dropped the `test:` target and the now-dead `%.json: bin/%` pattern rule. + +> [!NOTE] +> **`d3.js` and `d3.min.js` were not touched.** Their MD5s are unchanged from +> upstream v3.4.2. This is the single most useful security property of this +> fork: the shipped artefact is stock upstream, so published upstream analysis +> of D3 3.4.2 applies verbatim, and any diff against upstream is a finding. + +## Prioritization Signals + +- **1P OSS:** No. Third-party BSD-3-Clause OSS maintained as an internal + Google fork for AppSheet. +- **1P Proprietary Shipped Software:** Yes. `d3.min.js` is minified JavaScript + shipped verbatim to AppSheet editor and admin users. +- **High-Risk Code Surface:** Yes. The bundle contains a `new Function` + code-generation sink, an `innerHTML` sink, a `createContextualFragment` + sink, and a general-purpose XHR primitive with no origin controls. +- **Perimeter Exposure:** **Yes.** The consuming views are authenticated + AppSheet editor / admin pages served on the public Internet. +- **Data Sensitivity:** Medium–High. The hosting pages render customer app + schemas, view names, spreadsheet cell data, end-user identifiers and + activity timelines, and run with the operator's authenticated session. +- **Untrusted Input Handling:** Yes, but **indirect**. D3 is a DOM toolkit, not + a data renderer; in AppSheet the untrusted strings reach the DOM through + *nvd3's* use of `d3.selection.html()`, not through AppSheet's own D3 calls. +- **Business Value:** Substrate for AppSheet's usage-statistics, audit-log and + performance-diagnostics surfaces — the surfaces where one tenant's data is + displayed to another principal. + +### Cross-tenant framing + +Identical to nvd3's, because the hosting pages are identical: a malicious table +name, column name or spreadsheet value primarily renders in the creator's own +editor (self-XSS, low severity alone), but the same data is rendered for +**team/organisation admins** viewing apps they did not author and for **AppSheet +support and operations staff** investigating tickets. The realistic threat is +**stored XSS delivered from a low-privilege app creator to a higher-privilege +admin or support operator.** + +## Scanning Harness Prompts + +1. **`new Function` code generation in `src/dsv/dsv.js:35-37` (highest + priority).** `dsv.parse` compiles a row-accessor function at runtime: + ```js + var a = new Function("d", "return {" + row.map(function(name, i) { + return JSON.stringify(name) + ": d[" + i + "]"; + }).join(",") + "}"); + ``` + The generated function **body embeds the CSV/TSV header row**, i.e. attacker + influenceable text, and `JSON.stringify(name)` is the *only* thing separating + that text from generated source. Flag on two independent grounds: + - **CSP / Trusted Types incompatibility.** `new Function` requires + `unsafe-eval`. This is the same class of defect AppSheet already patched + out of `jquery-ajax-unobtrusive`. Under an enforcing policy this path + **fails closed** (DSV parsing throws) rather than executing. + - **Code generation from untrusted data.** Assess the quoting. `JSON.stringify` + escapes quotes and backslashes, but **does not escape U+2028 / U+2029**; + before ES2019 those were line terminators inside JS string literals and + would break out of the generated literal. Modern engines (ES2019+) permit + them in string literals, so there is no breakout on a current browser + matrix — **confirm the supported matrix rather than assuming it.** + - Exactly **one** `new Function` occurrence exists in the shipped `d3.js` + (verified by count). No AppSheet code calls `d3.csv` / `d3.tsv` / `dsv.parse`. +2. **`selection.html()` → `innerHTML` (`src/selection/html.js:6-9`).** Three + `innerHTML` writes (function-valued, null-clearing and constant), plus one + read in the getter branch. No escaping or sanitisation — by design, it is the + raw-HTML API. **This is the sink that actually matters in AppSheet**, because + nvd3 reaches it: nvd3's `nv.models.tooltip` default `contentGenerator` makes + three `.html()` calls on untrusted series keys and values, and that path is + *not* covered by nvd3's DOMPurify patch. **Cross-reference Priority Review + Area 1 of the nvd3 threat model; the sink lives here, the unsanitised caller + lives there.** +3. **`d3.html()` — fetch and parse HTML (`src/xhr/html.js`).** + ```js + function d3_html(request) { + var range = d3_document.createRange(); + range.selectNode(d3_document.body); + return range.createContextualFragment(request.responseText); + } + ``` + `createContextualFragment` is a named Trusted Types sink and will construct + live `<script>` elements from the response. Combined with prompt 4's absent + origin controls this is a fetch-and-inject primitive — the D3 analogue of + `nv.utils.pjax`, which the nvd3 model recommends deleting outright. +4. **`d3.xhr` — unrestricted outbound request (`src/xhr/xhr.js`).** + `request.open(method, url, true)` at `:94` with **no scheme or origin + allowlist**, caller-controlled `setRequestHeader` at `:96`, caller-controlled + `responseType` at `:98`, and an `XDomainRequest` fallback at `:24-26` for + legacy IE. Runs with the page's ambient session credentials. Underlies + `d3.json`, `d3.text`, `d3.xml`, `d3.html` and the DSV loaders. +5. **ReDoS review.** Four modules construct regular expressions: + `src/locale/time-format.js`, `src/dsv/dsv.js`, `src/selection/on.js`, + `src/selection/classed.js`. `time-format.js` is the one that parses + caller-supplied format and locale strings. +6. **Selector injection at the AppSheet call site.** + `d3.select('#' + $graph.attr('id'))` (`manageGraphs.js:117`, `:343`) builds a + CSS selector by string concatenation from a live DOM attribute. Determine + whether the graph element `id` can be influenced by app or table names. +7. **Build-artefact parity.** `d3.js` and `d3.min.js` are hand-committed, not + produced by CI. The build chain (`Makefile` → `smash ~0.0.12` + + `uglify-js 2.4.0`) is declared but those devDependencies are not installed or + lockfile-pinned, so the build is not readily reproducible. Current best + evidence of integrity is MD5 parity with the upstream v3.4.2 release — + re-verify that at every commit, because the minified artefact is what ships. +8. **Stale, unmaintained base.** v3.4.2 dates from February 2014. The v3 line + receives no upstream security maintenance. Enumerate advisories against + `d3@3.4.2` and treat "no fix available upstream" as the expected result. + +### Verified non-findings (do not spend review time here) + +- **Prototype pollution via `d3.map` / `d3.set`: mitigated.** + `src/arrays/map.js` prefixes every key with `"\0"` + (`var d3_map_prefix = "\0", // prevent collision with built-ins`) in `get`, + `set`, `has`, `remove` and `forEach`, so `__proto__`, `constructor` and + friends cannot reach the prototype chain. +- **`d3-color` ReDoS (CVE-2020-7746 class): not applicable.** That advisory + targets the standalone `d3-color` package of the v4+ modular era. In 3.4.2 + colour parsing lives in `src/color/*.js` and is a character-scan parser — + **no regular expressions at all** in that directory (verified). +- **jsdom / server-side DOM: removed.** Deleted in `b081036`; there is no + server-side execution path in this fork. +- **`lib/` never ships.** `lib/` contains vendored third-party material + (`colorbrewer/colorbrewer.js`, `science/science*.js`, plus LICENSE-only + directories for geographiclib, penner, polymaps, protovis, rhill-voronoi). + It is excluded by `.npmignore` and **verified absent from `npm pack` output**, + so it reaches neither `node_modules` nor the browser. It is not concatenated + into `d3.js` by the `Makefile`. + +## Entry Points and Untrusted Inputs + +| Entry Point | Type | Trusted? | Validation | +|---|---|---|---| +| `selection.html(value)` | JS API → `innerHTML` | No — reached by nvd3 with customer-authored series keys and values | **None by design.** Raw-HTML API | +| DSV header row → `dsv.parse` | Text → generated JS source | No — arbitrary CSV/TSV header text | `JSON.stringify` quoting only; not a security boundary. No AppSheet caller | +| DSV body rows | Text → JS objects | No | Character-scan tokeniser; values become plain object properties | +| `d3.xhr(url, …)` / `d3.json` / `d3.text` / `d3.xml` / `d3.html` | JS API → HTTP | Caller-supplied | **None.** No scheme or origin allowlist before `request.open` | +| `d3.html` response body | HTTP → DOM fragment | No — remote content | `createContextualFragment`, a Trusted Types sink. **No sanitisation** | +| Request headers via `xhr.header(name, value)` | JS API → HTTP | Caller-supplied | **None** | +| `d3.select(selector)` / `selectAll` | String → CSS selector | Caller-supplied; AppSheet concatenates a DOM `id` at `manageGraphs.js:117`, `:343` | **None.** Passed to `querySelector` | +| `selection.attr(name, value)` | JS API → DOM attribute | Caller-supplied | `d3.ns.qualify` resolves namespaces (incl. `xlink`); **no URL scheme filtering**, so `xlink:href` / `href` accept `javascript:` | +| `selection.style` / `.classed` / `.property` | JS API → DOM | Caller-supplied | **None** | +| `d3.time.format(specifier)` / locale strings | String → RegExp | Caller-supplied | Regex construction — see ReDoS prompt | + +## Trust Boundaries and Auth Assumptions + +- **Authentication:** None at the library layer. All four consuming views sit + behind AppSheet's authenticated editor/admin session; D3 runs with that + session's full ambient authority in the browser. +- **Authorization:** None at the library layer. D3 is a general-purpose DOM and + network toolkit and applies no policy of its own. +- **Implicit trust:** D3 assumes its caller has already decided what is safe. + Every API above is intentionally unguarded — `.html()` is *supposed* to write + raw HTML, `d3.xhr` is *supposed* to fetch any URL. **The security boundary is + the caller, and in AppSheet the relevant caller is nvd3, not AppSheet code.** +- **Boundary crossings:** + 1. App creator / end user authors a table name, column name, view name or + spreadsheet cell value. + 2. AppSheet server surfaces it in stats / audit-log / performance data. + 3. `manageGraphs.js` maps it into nvd3 `series[].key` and point `label`. + 4. nvd3's tooltip `contentGenerator` passes it to **`d3.selection.html()`**, + which assigns it to `innerHTML` — the sink in *this* repository. + Step 4 is the trust boundary. + +### Mitigating controls (verify, do not assume) + +- **CSP nonce.** All four views load the bundle with `nonce="@CspNonce"`, + indicating a nonce-based CSP. A correct nonce CSP blocks injected `<script>` + elements, and — relevant to prompt 1 — a policy without `unsafe-eval` blocks + `new Function` outright. This limits, but does not eliminate, impact: + overlay/UI-redress and markup-based data exfiltration remain, and the + protection evaporates if `script-src` carries a permissive fallback. +- **Trusted Types.** `innerHTML`, `createContextualFragment` and + `new Function` are all TT-enforced sinks. Under enforcement each **fails + closed** rather than executing. **Confirm whether + `require-trusted-types-for 'script'` is enforcing or report-only on each of + the four consuming routes**; if report-only, all three sinks are live. +- **Narrow current usage.** AppSheet exercises only `d3.select`, `d3.format` + and `d3.time`. The DSV codegen, the XHR family and `d3.html` have no caller + in the product today. +- **Artefact is stock upstream.** Because the fork did not modify `d3.js` / + `d3.min.js`, there is no Google-authored code in the shipped bytes to review + — a materially better position than the nvd3 fork, which inlines a pinned + DOMPurify. + +## Sensitive Data Paths + +| Data Type | Source | Destination | Protection | +|---|---|---|---| +| App / table / column / view names | Customer-authored app schema | nvd3 `series[].key` → `d3.selection.html()` → `innerHTML` | **None in D3.** DOMPurify on nvd3's legacy tooltip path only | +| Spreadsheet cell values | Customer / end-user data source | nvd3 point `label` / `value` → `.html()` / SVG `.text()` | `.text()` paths are safe; `.html()` paths unprotected | +| End-user activity and identifiers | AppSheet audit log | `_logDetailsPartial.cshtml` charts | Rendered in an admin/support session; same sinks | +| App usage statistics | AppSheet telemetry | `appStatsPartial.cshtml` charts | Same sinks; cross-tenant viewers possible | +| Expression / performance traces | AppSheet app compiler & runtime | `_perfDetailsPartial.cshtml` charts | Same sinks; traces may embed user formulas and column names | +| Session cookies / ambient credentials | Browser | Any `d3.xhr` destination | **None.** No origin allowlist; not currently invoked by AppSheet | + +## Privileged Actions + +| Action | Location | Guard | +|---|---|---| +| Compile a function from a string at runtime | `src/dsv/dsv.js:35` (`new Function`) | **None.** `JSON.stringify` quoting of header names only | +| Assign an arbitrary string to `innerHTML` | `src/selection/html.js:6-9` | **None** (raw-HTML API by design) | +| Parse remote HTML into a live DOM fragment | `src/xhr/html.js` (`createContextualFragment`) | **None** | +| Outbound HTTP request to a caller-supplied URL | `src/xhr/xhr.js:94` (`request.open`) | **None.** No scheme/origin allowlist | +| Set arbitrary request headers | `src/xhr/xhr.js:96` (`setRequestHeader`) | **None** | +| Cross-origin request via legacy fallback | `src/xhr/xhr.js:24-26` (`XDomainRequest`) | Scheme regex `/^(http(s)?:)?\/\//` only | +| Set arbitrary DOM attributes, incl. `xlink:href` / `href` | `src/selection/attr.js` via `d3.ns.qualify` | **None.** No URL scheme filtering | +| Construct RegExp from caller-supplied format/locale strings | `src/locale/time-format.js` | **None** | +| Resolve a CSS selector built by string concatenation | `manageGraphs.js:117`, `:343` (AppSheet side) | **None** | + +## Priority Review Areas + +1. **`new Function` code generation in `src/dsv/dsv.js` — the one Google-relevant + design defect in the shipped bundle.** It is unreachable from AppSheet today + and blocked by an enforcing CSP, but it is a runtime-codegen sink whose body + is built from untrusted header text, and it is precisely the pattern AppSheet + has already paid down elsewhere (`jquery-ajax-unobtrusive`). **Recommendation: + since AppSheet never calls `d3.csv` / `d3.tsv`, consider deleting `src/dsv/` + from the fork and rebuilding** — the same "delete unused attack surface" + reasoning the nvd3 model applies to `nv.utils.pjax`, and consistent with this + fork's existing deletion-only posture. Note this would change `d3.min.js`, + forfeiting the upstream-parity property, so weigh it deliberately. +2. **`selection.html()` reached by nvd3's unsanitised tooltip path.** The sink is + in this repository; the vulnerable caller is in the nvd3 fork. Neither + document is actionable alone. Triage these two assets as a single unit, and + fix at the nvd3 end (`getTooltipContainer` and the default + `contentGenerator`), not here — D3's `.html()` is behaving as specified. +3. **`d3.html` + `d3.xhr` as a fetch-and-inject primitive.** No origin controls, + a Trusted Types sink on the parse side, ambient credentials on the fetch + side, and no AppSheet caller. Same disposition question as `src/dsv/`: + delete, or accept-and-document. +4. **Confirm CSP `unsafe-eval` absence and Trusted Types enforcement mode on the + four consuming routes.** These two controls are what currently bound the + impact of items 1–3. If TT is report-only, all three sinks are live. +5. **Decade-old, end-of-life base.** v3.4.2 (2014) on the EOL v3 line, carrying + nvd3 v1.1.15b (2013) above it. No upstream security maintenance exists for + either. The durable remediation is migrating these four legacy views off the + nvd3/D3-v3 stack entirely; until then, both forks are frozen liabilities. +6. **Build reproducibility.** Establish that `d3.js` / `d3.min.js` can be + regenerated from `src/` — pin `smash` and `uglify-js`, or record that the + artefacts are accepted as upstream-verbatim and enforce that with an MD5 + check in review. +7. **Selector construction at `manageGraphs.js:117` / `:343`.** AppSheet-side, + but it is the only place AppSheet feeds a concatenated string into a D3 API. + Confirm the graph `id` cannot carry attacker-controlled characters. + +## Out of Scope + +- **nvd3.** Tracked separately at + `https://gnocchi-internal.googlesource.com/third_party/nvd3` with its own + `THREAT_MODEL.md`. Its tooltip sanitisation gap is the principal way untrusted + data reaches D3's `innerHTML` sink, so the two documents are complementary and + should be triaged together — but nvd3's own code must be reviewed there. +- **`jeenee/Nirvana/Scripts/app/plugins/nv.d3.indentedTree.js`** — a separately + vendored file from a *different* nvd3 version, not produced by either fork. + Review on the `jeenee` side. +- **`lib/`** — vendored demo dependencies (colorbrewer, science.js) and + LICENSE-only directories. Excluded by `.npmignore`, verified absent from + `npm pack`, not referenced by the `Makefile`, never served. +- **`Makefile`, `bin/start`, `bin/uglify`, and the `smash` / `uglify-js` + devDependencies** — build-time only. No CI executes them for this repository + and they produce no automatically served artefact. Retained so the build + remains auditable; see Priority Review Area 6. +- **`src/start.js` / `src/end.js`** — the IIFE wrapper and version stamp + generated from `package.json`. +- **Deleted upstream code** — `index.js` (jsdom shim), `test/`, `bower.json`, + `component.json`, `composer.json`, `.travis.yml`. Absent from `main`; review + only if resurrected. +- **Server-side generation and authorisation of the app-stats, audit-log and + performance data feeds**, `manageGraphs.js`, the webpack `d3Init.ts` shim, and + the CSP / Trusted Types middleware. All live in the `jeenee` repository and + must be reviewed there; referenced here only as context and as mitigating + controls to be verified.