| # Security Threat Model: nvd3 (AppSheet Fork) |
| |
| ## Asset Definition & Scope |
| |
| - **Component:** nvd3 — a library of reusable, composable SVG chart components |
| built on D3.js v3. Upstream is `novus/nvd3`; per `METADATA` the fork's base |
| is upstream **v1.1.15b, last upgraded 2013-12-18**, i.e. a decade-old |
| snapshot that no longer tracks upstream. |
| - **Repository:** Git-on-Borg |
| (`https://gnocchi-internal.googlesource.com/third_party/nvd3`, branch |
| `main`). |
| - **Fork identity:** `package.json` declares `"version": "0.0.1"`, and the |
| Grunt uglify banner (`GruntFile.js`) stamps the artefacts accordingly — the |
| built `nv.d3.min.js` begins `/*! nvd3 - v0.0.1 - 2023-09-13 */`. The version |
| string is deliberately detached from upstream numbering, so upstream-version |
| matching by vulnerability scanners will not work on this asset. |
| - **Scope:** `src/` (the authored sources), and the concatenated/minified build |
| artefacts `nv.d3.js` and `nv.d3.min.js` produced from them by `GruntFile.js` |
| / `Makefile`. `nv.d3.css` / `.min.css` are presentational. |
| |
| > [!IMPORTANT] |
| > **This fork reaches production as a *vendored copy*, not as an npm/bower |
| > dependency.** No `package.json` in AppSheet's `jeenee` monorepo declares a |
| > dependency on `nvd3` — dependency-graph tooling will report this repository as |
| > unused. It is not. The build artefact in this repository is **byte-identical** |
| > (verified by MD5, `ba459888d567a9f5375c7d90c698e9cf`) to the file checked into |
| > the AppSheet web frontend at `jeenee/Nirvana/Scripts/app/plugins/nv.d3.min.js`, |
| > which is served to authenticated AppSheet users. This repository is the |
| > confirmed build source of live, Internet-facing production JavaScript, and |
| > findings here are directly exploitable findings in AppSheet. |
| |
| **Google-authored modification.** The single substantive fork change (commits |
| `f07d57e` "[nwd] Patching nv.d3 trusted types violations" and `189db60` "[nwd] |
| Fixing a bug - DOMPurify was not imported", kjamali@, Sept 2023) **inlines |
| DOMPurify 3.0.5 into the bundle** as `src/purify.js` and calls it from one |
| tooltip sink. Upstream nvd3 has no DOMPurify dependency; `src/purify.js` is |
| concatenated into `nv.d3.js` ahead of the library proper. This was done to |
| satisfy AppSheet's Trusted Types enforcement, not as a general XSS hardening |
| pass — see Priority Review Area 1. |
| |
| **Production consumers.** The vendored `nv.d3.min.js` is loaded by four live |
| legacy Razor views in `jeenee`, all with a CSP nonce: |
| |
| | View | Line | Surface | |
| |---|---|---| |
| | `V3/MainServer/Views/template/_showTablePartial.cshtml` | 91 | App editor — table/column detail | |
| | `V3/MainServer/Views/Shared/_logDetailsPartial.cshtml` | 76 | Audit-log detail | |
| | `V3/MainServer/Views/Shared/_perfDetailsPartial.cshtml` | 56 | Performance / expression trace detail | |
| | `V3/MainServer/Views/template/commoneditor/appStatsPartial.cshtml` | 102 | App usage statistics | |
| |
| The driver is `jeenee/Nirvana/Scripts/sp-theme/manageGraphs.js` (loaded by |
| `Views/template/AppStats.cshtml:49` and `Views/manage/ViewLog.cshtml:194`). |
| It builds chart data at `manageGraphs.js:72-88` — series `key` comes from |
| `data[i].series` and point `label` from the keys of `data[i].data` — then calls |
| `nv.addGraph(...)` at `manageGraphs.js:114` (and again at `:336` for |
| `showTimeGraph`). Those series keys and point labels derive from **AppSheet app, |
| table, column and view names and from user spreadsheet data**, i.e. from |
| attacker-influenceable strings. |
| |
| ## Prioritization Signals |
| |
| - **1P OSS:** No. Third-party Apache-2.0 OSS maintained as an internal Google |
| fork for AppSheet. |
| - **1P Proprietary Shipped Software:** Yes. `nv.d3.min.js` is minified |
| JavaScript shipped verbatim to AppSheet editor and admin users. |
| - **High-Risk Code Surface:** Yes. The library's core rendering idiom is |
| building HTML strings and assigning them to `innerHTML`, and it bundles a |
| pinned 2023 copy of a security-critical sanitiser (DOMPurify 3.0.5) that |
| receives no upstream updates. |
| - **Perimeter Exposure:** **Yes.** The consuming views are authenticated |
| AppSheet editor / admin pages served on the public Internet. |
| - **Data Sensitivity:** Medium–High. Chart labels are derived from customer |
| app schemas, view names and spreadsheet cell data; the audit-log and |
| app-stats surfaces additionally display end-user identifiers and activity |
| timelines. The library also renders inside pages that carry the operator's |
| authenticated session. |
| - **Untrusted Input Handling:** Yes. Series keys, point labels, axis label |
| text, tooltip header/value strings, and the optional `footer` field all flow |
| from application data into the DOM. |
| - **Business Value:** Renders AppSheet's usage-statistics, audit-log and |
| performance-diagnostics surfaces. These are precisely the surfaces where |
| **one tenant's data is displayed to another principal** — see the |
| cross-tenant framing below. |
| |
| ### Cross-tenant framing (stated honestly) |
| |
| A malicious table name, column name or spreadsheet value primarily renders in |
| **the creator's own editor**, which is self-XSS and low severity on its own. The |
| escalation path is that the same data is rendered on shared and delegated |
| surfaces: |
| |
| - **team/organisation admins** viewing app statistics and audit logs for apps |
| they did not author; |
| - **AppSheet support and operations staff** opening a customer's app stats, |
| audit log or performance trace while investigating a ticket — a |
| cross-tenant, higher-privilege viewer. |
| |
| So the realistic threat is **stored XSS delivered from a low-privilege app |
| creator to a higher-privilege admin or support operator**, not drive-by attack |
| of anonymous users. That is the severity this asset should be triaged at. |
| |
| ## Scanning Harness Prompts |
| |
| 1. **Incomplete DOMPurify coverage in `src/tooltip.js` (highest priority).** |
| The file contains **two** tooltip implementations and the fix was applied to |
| only one: |
| - `nv.tooltip.show()` (the legacy helper) **is** patched: |
| `var safeContent = DOMPurify.sanitize(content, {RETURN_TRUSTED_TYPE: true}); |
| container.innerHTML = safeContent;` (`src/tooltip.js:340-341`, built |
| `nv.d3.js:2339-2340`). |
| - `nv.models.tooltip()` (the "new way", per the file's own header comment) |
| is **not**: its private `getTooltipContainer(newContent)` does a bare |
| `container.node().innerHTML = newContent;` (`src/tooltip.js:167`, built |
| `nv.d3.js:2166`) with **no DOMPurify call anywhere in that code path**. |
| Its default `contentGenerator` builds the markup with D3 `.html()` |
| calls — `.html(headerFormatter(d.value))` (`:83`), |
| `.html(function(p) {return p.key})` (`:101`), |
| `.html(function(p,i) { return valueFormatter(p.value,i) })` (`:104`) — |
| serialises via `table.node().outerHTML`, then **string-concatenates an |
| unescaped footer**: `html += "<div class='footer'>" + d.footer + "</div>"` |
| (`:119-120`). Flag every one of these as an XSS sink. |
| 2. **Reachability of the unsanitised sink.** `nv.models.tooltip` is instantiated |
| by `nv.interactiveGuideline` (`src/interactiveLayer.js:11`, exposed as |
| `layer.tooltip` at `:157`) and invoked as |
| `interactiveLayer.tooltip.position(...).data({value, series})()` by |
| `src/models/lineChart.js:276`, `src/models/stackedAreaChart.js:401` and |
| `src/models/cumulativeLineChart.js:498` — but only when the consumer calls |
| `chart.useInteractiveGuideline(true)`. Scanners should treat this as a live, |
| reachable sink in the shipped bundle and as a public API |
| (`nv.models.tooltip` is exported on `window.nv.models`). |
| 3. **Stale bundled sanitiser.** `src/purify.js` is a frozen copy of **DOMPurify |
| 3.0.5** (`DOMPurify.version = '3.0.5'`, `src/purify.js:319`), released 2023. |
| It is inlined into `nv.d3.js`/`nv.d3.min.js`, so it will never be updated by |
| `npm audit`, Dependabot, or any lockfile-based tooling, and it is invisible to |
| SCA that keys off `package.json`. Check 3.0.5 against subsequently disclosed |
| DOMPurify bypasses and mutation-XSS issues, and flag the absence of any |
| update mechanism as a finding in its own right. |
| 4. **`nv.utils.pjax` — network fetch plus history rewrite.** |
| `src/utils.js:87-105` binds click handlers that call |
| `history.pushState(this.href, …)` and then `d3.html(href, …)`, parsing the |
| response and splicing a selected node into the live DOM via |
| `target.parentNode.replaceChild(...)`. There is no origin or scheme check on |
| `href`. This is a general-purpose fetch-and-inject primitive with no |
| relationship to charting. |
| 5. **SVG identifier and selector construction.** Several models build element |
| IDs and `clip-path` URL references by string concatenation from a chart `id` |
| — e.g. `src/models/historicalBar.js:95,98,102` |
| (`'nv-chart-clip-path-' + id`), `src/models/line.js:99,101` |
| (`'url(#nv-edge-clip-' + scatter.id() + ')'`), and the equivalents in |
| `multiBar.js`, `multiBarTimeSeries.js`, `ohlcBar.js`, `scatter.js`, |
| `lineWithFisheye.js`. Those IDs are also fed back into |
| `wrap.select('#…')` selectors. Verify that no caller can set a chart `id` to |
| an attacker-controlled string and thereby break out of the ID/selector |
| context. |
| 6. **Build-artefact parity.** `nv.d3.js` and `nv.d3.min.js` are hand-committed, |
| not produced by CI. Confirm they are faithful builds of `src/` at every |
| commit; the minified artefact is what actually ships. |
| |
| ## Entry Points and Untrusted Inputs |
| |
| | Entry Point | Type | Trusted? | Validation | |
| |---|---|---|---| |
| | Chart datum `series[].key` | JS data → DOM | No — derives from app/table/column/view names (`manageGraphs.js:76`) | `nv.models.tooltip` default `contentGenerator`: `.html(function(p){return p.key})` — **none**. Legacy `nv.tooltip.show` path: DOMPurify | |
| | Chart datum point `label` / `value` | JS data → DOM | No — derives from user spreadsheet data (`manageGraphs.js:81-82`) | Passed through `valueFormatter` then `.html(...)` — **no escaping** in `nv.models.tooltip` | |
| | `d.value` (tooltip header) | JS data → DOM | No | `.html(headerFormatter(d.value))` — `headerFormatter` defaults to identity, **no escaping** | |
| | `d.footer` | JS data → DOM | No | String-concatenated into `"<div class='footer'>" + d.footer + "</div>"` — **no escaping** | |
| | `chart.tooltipContent(fn)` / `contentGenerator(fn)` | JS API | Caller-supplied | Caller-defined HTML string; the default in e.g. `src/models/multiBarChart.js:28-31` concatenates `key`, `x`, `y` into `<h3>`/`<p>` markup | |
| | `chart.xAxis.axisLabel(text)` | JS API → SVG | No — `options.xAxisName` (`manageGraphs.js:106,108`) | Rendered via D3 `.text()` on an SVG `<text>` node in `src/models/axis.js` (**not** `.html()`), so this is text-context and comparatively safe | |
| | `nv.utils.pjax(links, content)` link `href` | DOM attribute → HTTP | No | **None.** No scheme or origin allowlist before `d3.html(href, …)` | |
| | Chart `id` | JS API → SVG ids and selectors | Caller-supplied | Concatenated into `clipPath` `id` attributes and `#…` selectors without escaping | |
| |
| ## Trust Boundaries and Auth Assumptions |
| |
| - **Authentication:** None at the library layer. All four consuming views are |
| behind AppSheet's authenticated editor/admin session; the library runs with |
| that session's full ambient authority in the browser. |
| - **Authorization:** None at the library layer. The library renders whatever |
| data the hosting page hands it and makes no distinction between |
| first-party-generated labels and customer-authored strings. |
| - **Implicit trust:** The library assumes chart data — series keys, point |
| labels, header values, footers — is trustworthy HTML. In AppSheet that |
| assumption is false: those strings originate from app creators and end-user |
| spreadsheet content. |
| - **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 `series[].key` and point `label`. |
| 4. nvd3 renders it into the DOM of an **editor, team-admin or support |
| operator's** authenticated page. |
| Step 4 is the trust boundary. The sanitisation gap in `nv.models.tooltip` is |
| exactly at that 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. This limits, but does not eliminate, impact: injected |
| markup, event-handler attributes rejected by CSP still leave overlay/UI-redress |
| and data-exfiltration-via-markup avenues open, and the protection evaporates |
| if `script-src` carries a permissive fallback. |
| - **Trusted Types.** The 2023 fork exists to satisfy TT enforcement. Note that |
| the bare `container.node().innerHTML = newContent` in `getTooltipContainer` |
| would be *blocked* by TT enforcement rather than sanitised — meaning under |
| full TT the unsanitised path fails closed (tooltip breaks) rather than |
| executing. **Confirm whether TT is enforced (not report-only) on each of the |
| four consuming routes**; if it is report-only, the sink is live. |
| - **Current AppSheet usage narrows exposure.** The only nvd3 chart model |
| AppSheet instantiates is `nv.models.multiBarChart` |
| (`manageGraphs.js:46`, `V3/MainServer/Views/template/commoneditor/_perfDisplayPartial.cshtml:148`), |
| whose tooltip routes through the **DOMPurify-sanitised** `nv.tooltip.show` |
| (`src/models/multiBarChart.js:72`). AppSheet does **not** currently call |
| `useInteractiveGuideline(true)`, so the unsanitised `nv.models.tooltip` path |
| is not exercised by today's call sites. It remains shipped, exported, and one |
| line of caller code away from being live — which is why it is still ranked |
| first below. |
| |
| ## Sensitive Data Paths |
| |
| | Data Type | Source | Destination | Protection | |
| |---|---|---|---| |
| | App / table / column / view names | Customer-authored app schema | `series[].key` → tooltip DOM | DOMPurify on the `nv.tooltip.show` path; **none** on the `nv.models.tooltip` path | |
| | Spreadsheet cell values | Customer / end-user data source | Point `label` / `value` → tooltip and axis DOM | Axis labels use `.text()` (safe); tooltip values use `.html()` (unprotected on the `nv.models.tooltip` path) | |
| | End-user activity and identifiers | AppSheet audit log | `_logDetailsPartial.cshtml` charts | Rendered in an admin/support session; same tooltip sinks | |
| | App usage statistics | AppSheet telemetry | `appStatsPartial.cshtml` / `AppStats.cshtml` charts | Same tooltip sinks; cross-tenant viewers possible | |
| | Expression / performance traces | AppSheet app compiler & runtime | `_perfDetailsPartial.cshtml` charts | Same tooltip sinks; traces may embed user formulas and column names | |
| |
| ## Privileged Actions |
| |
| | Action | Location | Guard | |
| |---|---|---| |
| | Assign untrusted string to `innerHTML` | `src/tooltip.js` : `nv.models.tooltip` → `getTooltipContainer` | **None** | |
| | Assign string to `innerHTML` after sanitisation | `src/tooltip.js` : `nv.tooltip.show` | `DOMPurify.sanitize(content, {RETURN_TRUSTED_TYPE: true})` | |
| | Render untrusted values as HTML via D3 | `src/tooltip.js` : `nv.models.tooltip` default `contentGenerator` (`.html()` × 3, plus footer concatenation) | **None** | |
| | Outbound HTTP fetch of a DOM-supplied URL and DOM replacement | `src/utils.js` : `nv.utils.pjax` → `load` (`d3.html`, `replaceChild`) | **None.** No scheme/origin allowlist | |
| | Browser history mutation | `src/utils.js` : `nv.utils.pjax` (`history.pushState`) | **None** | |
| | Bundled HTML sanitiser (security-critical dependency) | `src/purify.js` (DOMPurify 3.0.5), inlined into `nv.d3.js` / `nv.d3.min.js` | Pinned; no update path, invisible to SCA | |
| | SVG `id` / `clip-path` / selector construction from a chart `id` | `src/models/{historicalBar,line,multiBar,multiBarTimeSeries,ohlcBar,scatter,lineWithFisheye,discreteBarChart}.js` | String concatenation, no escaping | |
| |
| ## Priority Review Areas |
| |
| 1. **`nv.models.tooltip` was left out of the DOMPurify fix |
| (`src/tooltip.js` → `getTooltipContainer` and the default |
| `contentGenerator`).** The 2023 patch sanitised the legacy `nv.tooltip.show` |
| helper and stopped there, even though the file's own header comment calls |
| `nv.models.tooltip` "the updated, new way to render tooltips". The result is |
| a shipped, exported tooltip renderer that takes series keys, point values, |
| header values and a footer straight into `.html()` and `innerHTML` with no |
| escaping and no sanitiser. It is invoked in the bundle by |
| `lineChart`/`stackedAreaChart`/`cumulativeLineChart` under |
| `useInteractiveGuideline(true)`. **Accuracy note:** AppSheet's current call |
| sites use only `multiBarChart`, which routes through the *sanitised* helper, |
| so this is a **latent** rather than presently-exercised sink — but it is one |
| `useInteractiveGuideline(true)` away, and the same file already proves the |
| project knows this data is untrusted. Fix by routing |
| `getTooltipContainer` through `DOMPurify.sanitize(..., {RETURN_TRUSTED_TYPE: true})` |
| and replacing the `.html()` calls with `.text()`. |
| 2. **Stale, inlined DOMPurify 3.0.5 with no update path (`src/purify.js`).** A |
| security control frozen at a 2023 release and concatenated into a build |
| artefact is invisible to every dependency scanner AppSheet runs. Establish |
| ownership and an update cadence, check 3.0.5 against subsequently published |
| bypasses, and prefer consuming DOMPurify as a real, version-pinned dependency |
| over an inlined copy. |
| 3. **`nv.utils.pjax` (`src/utils.js`).** A fetch-and-inject plus |
| `history.pushState` primitive with no origin validation, shipped inside a |
| charting library. Determine reachability: no AppSheet code references |
| `nv.utils.pjax`, and it serves no charting purpose. **Recommendation: delete |
| it from the fork** — it is pure attack surface with zero product value. |
| 4. **Cross-tenant exposure of the audit-log, app-stats and performance views.** |
| Confirm which principals can view another tenant's charts (team/org admins, |
| AppSheet support/ops tooling) and treat any tooltip XSS on those routes as |
| privilege-escalating stored XSS rather than self-XSS. |
| 5. **Effective CSP and Trusted Types enforcement mode on the four consuming |
| routes.** Verify `script-src` has no permissive fallback and determine |
| whether `require-trusted-types-for 'script'` is enforcing or report-only. |
| These two controls are what currently bound the impact of every sink above. |
| 6. **SVG identifier and selector construction.** Audit chart `id` propagation |
| into `clipPath` `id` attributes, `url(#…)` references, and `d3.select('#'+id)` |
| selectors across `src/models/` for injection or selector-confusion. |
| 7. **Decade-old upstream base.** The fork is built on nvd3 v1.1.15b (2013) and |
| D3 v3 (`bower.json` pins `d3 ~3.3.5`). Neither receives security |
| maintenance. Evaluate migrating these four legacy views off nvd3 entirely as |
| the durable remediation. |
| |
| ## Out of Scope |
| |
| - **`jeenee/Nirvana/Scripts/app/plugins/nv.d3.indentedTree.js` is NOT produced |
| by this fork.** Its header comment pins it to upstream |
| `https://github.com/novus/nvd3/blob/v1.2.1/src/models/indentedTree.js` — a |
| *different* upstream version from this repository's v1.1.15b base — and it is |
| a separately vendored, standalone file in the `jeenee` repository. It is |
| loaded alongside `nv.d3.min.js` by `_showTablePartial.cshtml:92`, |
| `_logDetailsPartial.cshtml:77` and `_perfDetailsPartial.cshtml:57`, and |
| instantiated as `nv.models.indentedTree()` at `_showTablePartial.cshtml:207` |
| and `_perfDetailsPartial.cshtml:352`. This repository does contain a |
| `src/models/indentedTree.js`, but it is **not** the file that ships. |
| **`nv.d3.indentedTree.js` must be reviewed on the `jeenee` side, not here.** |
| - `deprecated/`, `examples/`, `test/`, `lib/` — sample pages, fixtures and |
| vendored demo dependencies with no production code path. |
| - `src/models/backup/` — superseded model sources retained in-tree; not |
| concatenated into the build. |
| - `GruntFile.js`, `Makefile`, `build.bat`, `.jshintrc` and the Grunt/Babel |
| `devDependencies` — build-time only, not executed in CI for this repository, |
| and producing no automatically served artefact. |
| - `nv.d3.css` / `nv.d3.min.css` — stylesheets with no scripting surface. |
| - D3.js itself. The library requires a global `d3` (v3), supplied separately by |
| AppSheet; it is not vendored in this repository. |
| - Server-side generation and authorisation of the app-stats, audit-log and |
| performance data feeds, `manageGraphs.js`, and the CSP/Trusted Types |
| middleware. All live in the `jeenee` repository and must be reviewed there; |
| they are referenced here only as context and as mitigating controls to be |
| verified. |