Add Fortify scanner THREAT_MODEL.md to third_party/element-resize-detector repo Bug: b/548455614 Change-Id: I102ac0c02d1a755498e0665ad8b4b6dbc7c5d954 Reviewed-on: https://gnocchi-internal-review.git.corp.google.com/c/third_party/element-resize-detector/+/316629 Reviewed-by: Trevor Ryland <tryland@google.com>
diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md new file mode 100644 index 0000000..7dee714 --- /dev/null +++ b/THREAT_MODEL.md
@@ -0,0 +1,264 @@ +# Security Threat Model: element-resize-detector (AppSheet Fork) + +## Asset Definition & Scope + +`element-resize-detector` is a small browser-only JavaScript library +(~1,550 lines across `src/`) that emits resize events for arbitrary DOM +elements, in browsers or situations where `ResizeObserver` is unavailable or +undesirable. Repository: +`sso://gnocchi-internal/third_party/element-resize-detector`, branch `main` +(mirrored at `appsheet-third-party/element-resize-detector`). + +- **Upstream:** `https://github.com/wnr/element-resize-detector`, npm version + `1.2.4`, upstream revision `bae322eff9057a532ec42e324f0ef05d4e509f5f` + (`last_upgrade_date` 2021-12-14, per `METADATA`). +- **Scope:** `src/` (the shipped implementation) and `dist/` (the prebuilt + browserify bundles `element-resize-detector.js` and + `element-resize-detector.min.js`). Build tooling (`Gruntfile.js`, + `karma.conf.js`), `test/`, and `examples/` are development-only. +- **How it works:** It detects resizes by *injecting hidden DOM into the + observed element* and listening for side effects. Two strategies exist: + - **`object` strategy (default):** appends a hidden + `<object type="text/html" data="about:blank">` — a same-origin nested + browsing context — into the target element and subscribes to the inner + window's native `resize` event. + - **`scroll` strategy:** appends a tree of absolutely-positioned, + overflow-scrolling `<div>`s plus a `<style>` element injected into + `document.head`, and infers size changes from scroll events and CSS + animation start events. +- **Deployment context:** Runs in the end user's browser as part of the + AppSheet web frontend. It is not used server-side. + +## Consumption in AppSheet + +This fork is a **transitive** dependency, force-resolved to the Google fork via +an `overrides` entry in `jeenee/Nirvana/Content/package.json`: + +```json +"react-sizeme": { + "element-resize-detector": "git+https://appsheet-third-party.googlesource.com/element-resize-detector#4a11fdb85cfe9f03d944f23b9f0231d454e02b53" +} +``` + +`package-lock.json` resolves `node_modules/element-resize-detector` to that +exact commit (version `1.2.4`). Its only consumer is `react-sizeme@3.0.2`, whose +`withSize` higher-order component is imported by +`Nirvana/Content/components/app/primary/AdjustableDash.tsx` — the resizable +dashboard layout in the AppSheet app runtime. The library is therefore genuinely +executed in production end-user sessions, on customer app pages. + +## The AppSheet Customization + +Two commits, both Trusted Types remediation +(`go/trusted-types-appsheet`), no functional divergence from upstream 1.2.4: + +| Commit | Change | +|---|---| +| `3a3dda5` "Initial import" | Import of upstream 1.2.4 **with the Trusted Types patch already applied to `src/`**: in `src/detection-strategy/scroll.js`, `injectScrollStyle`'s inner `injectStyle` sets `styleElement.textContent = style` instead of upstream's `styleElement.innerHTML = style`. | +| `4a11fdb` "Updated the dist folder" (HEAD) | Rebuilt `dist/element-resize-detector.js` and `dist/element-resize-detector.min.js`. The commit message notes the previously imported fork shipped **unpatched** built files — i.e. `src/` was fixed but the bundle consumers actually load was not. This commit closed that gap. | + +One upstream `innerHTML` sink deliberately remains, in +`src/browser-detector.js`: the legacy IE-version probe assigns a **static** +conditional-comment string +(`div.innerHTML = "<!--[if gt IE " + (++v) + "]><i></i><![endif]-->"`) to a +detached `<div>`. The interpolated value is a loop counter, not user data. + +## Prioritization Signals + +- **1P OSS:** No (Third-party OSS maintained as an internal customized fork + for Google AppSheet) +- **1P Proprietary Shipped Software:** Yes (Bundled and minified into the + AppSheet frontend JavaScript shipped to browsers) +- **High-Risk Code Surface:** Yes — the library's core mechanism is + programmatic DOM injection into caller-supplied elements, creation of a + nested browsing context (`<object>`), injection of a `<style>` element into + `document.head`, cross-document event-listener registration, mutation of the + host page's inline styles, and `setTimeout` polling loops. +- **Perimeter Exposure:** Yes (AppSheet is a 1P service exposed to the public + Internet and customers; this code runs on customer-facing app pages) +- **Data Sensitivity:** Low — the library handles element geometry + (`offsetWidth`, `offsetHeight`, computed style) and internally generated + numeric IDs. It never reads element text, form values, or app data. Its + `reporter` may `console.warn` a *reference* to a DOM element when styles + conflict, which could surface page structure in browser logs. +- **Untrusted Input Handling:** Indirect / low. Customer-authored app data + never flows into this library as a string. All CSS it generates is built + from hardcoded literals and numbers derived from the layout engine. The + untrusted-input question is instead about *which element* and *which + options* the host application hands it. +- **Business Value:** Supports the resizable dashboard layout in the AppSheet + app runtime. Security impact is bounded: realistic failure modes are + same-origin script injection *if* the injected `<object>` URL ever became + caller-influenced, CSS/UI-redress issues from injected hidden elements, and + listener/detached-DOM leaks causing browser-side denial of service in + long-lived sessions. + +## Scanning Harness Prompts + +1. **Nested browsing context creation (`src/detection-strategy/object.js`).** + The `injectObject` path builds `document.createElement("object")`, sets + `object.type = "text/html"` and `object.data = "about:blank"`, then appends + it into the caller's element. `about:blank` inherits the embedding page's + origin, so its `contentDocument` is fully same-origin. Verify `object.data` + can never be influenced by caller options, element attributes, or app + configuration — a controllable `data` value here is a direct same-origin + script-injection primitive. Also verify the element is not given + `allow-scripts`-equivalent capabilities beyond the default and that + `object.tabIndex = -1` / `aria-hidden="true"` remain set so the hidden node + cannot be focused or announced. +2. **DOM / style injection sinks.** Audit every write to `innerHTML`, + `textContent`, `style.cssText`, `className`, and `setAttribute` in `src/`: + - `scroll.js:injectScrollStyle` → `styleElement.textContent` (the patched + sink — confirm it has not regressed to `innerHTML` and that `dist/` + matches `src/`). + - `scroll.js:getScrollbarSizes`, `scroll.js:storeStartSize`, + `scroll.js:initListeners`, and `object.js:injectObject` → repeated + `element.style.cssText = buildCssTextString([...])` assignments. + Confirm every interpolated value is either a hardcoded literal or a + number originating from `offsetWidth`/`offsetHeight`, never a string + from the caller. + - `browser-detector.js` → the remaining static-string `innerHTML` + assignment. Confirm the interpolated value stays a loop counter. + - `scroll.js` CSS rule construction interpolates `containerClass`; confirm + `detectionContainerClass` remains the hardcoded + `"erd_scroll_detection_container"` constant and never becomes an option. +3. **Event-listener and timer leaks (denial of service).** Trace + `element-resize-detector.js:listenTo` / + `element-resize-detector.js:uninstall` against + `object.js:addListener`/`object.js:uninstall` and + `scroll.js:addListener`/`scroll.js:uninstall`. Check that: every + `addEventListener`/`attachEvent` has a matching removal; the + `resize` listener registered on + `object.contentDocument.defaultView` is detached before the `<object>` node + is removed; the `state.checkForObjectDocumentTimeoutId` polling loop in + `object.js:onObjectLoad`→`getDocument` is always cleared; and + `listener-handler.js:removeAllListeners` plus + `state-handler.js` do not retain references to detached elements. Unbounded + growth here is a realistic client-side DoS in long-lived AppSheet sessions + that mount and unmount dashboards repeatedly. +4. **Host-page style mutation / UI redress.** Both strategies call + `element.style.setProperty("position", "relative", important)` and may zero + out `top`/`right`/`bottom`/`left` on statically positioned targets + (`object.js:alterPositionStyles`, `scroll.js:alterPositionStyles`). Review + whether the `important: true` option, combined with the injected + absolutely-positioned containers, can be used to overlay or displace + interactive UI (clickjacking / UI-redress) on a customer app page. +5. **Reentrancy and unbounded recursion.** The scroll strategy re-enters + `updateChildSizes`/`positionScrollbars` from its own scroll and + animationstart handlers. Check the batch processor + (`batch-processor@1.0.0`) and the `updateDetectorElements` path for + feedback loops that could spin the main thread when an element is resized by + its own resize listener. +6. **`dist/` vs `src/` drift.** `dist/` is a checked-in browserify bundle and + is what downstream tooling may load. Commit `4a11fdb` exists precisely + because these fell out of sync and shipped an unpatched `innerHTML`. Flag + any state where `dist/` does not reflect the current `src/`. + +## Entry Points and Untrusted Inputs + +| Entry Point | Type | Trusted? | Validation | +|---|---|---|---| +| `elementResizeDetectorMaker(options)` (`src/element-resize-detector.js`) | Factory / configuration | Yes (called by `react-sizeme`, first-party bundle code) | `getOption` applies defaults; `strategy` is compared against the literals `"scroll"`/`"object"` and silently falls back to `object`; `important`, `debug`, `callOnAdd` coerced with `!!` | +| `erd.listenTo([options,] elements, callback)` | DOM element handle + callback | Yes (first-party caller supplies the element) | `isElement()` checks `nodeType === 1`; `isCollection()` normalises array-likes; non-elements are rejected via `reporter.error` | +| `erd.uninstall(elements)` / `erd.removeListener` / `erd.removeAllListeners` | Teardown API | Yes | Element state existence checked before teardown | +| `options.idHandler` / `options.reporter` / `options.batchProcessor` | Injected strategy objects (callable hooks) | Yes — but they are arbitrary caller-supplied code executed inside the library | None. A hostile or buggy implementation runs with full page privileges; safety rests on these being first-party. | +| Browser layout values (`offsetWidth`, `offsetHeight`, `getComputedStyle`) | Ambient DOM / layout engine | Yes | Used as numbers in CSS pixel strings; no string interpolation from user content | +| Native `resize` / `scroll` / `animationstart` events on injected nodes | Browser event stream | Yes | Handlers only read geometry from the injected nodes | + +**Net assessment:** there is no path by which customer-authored app data reaches +this library as a string. The genuine trust dependency is that the *host +application* passes only its own elements and its own option objects. + +## Trust Boundaries and Auth Assumptions + +- **Authentication**: None. Client-side DOM utility with no network or + credential handling. +- **Authorization**: None. Any script in the page can call the exported + factory; there is nothing to authorise. +- **Implicit trust**: (a) The caller supplies elements and options it owns. + (b) `about:blank` is and remains the only URL loaded into the injected + `<object>`. (c) Injected hidden nodes are inert and will not be traversed or + styled by application CSS. (d) `document.head` is available and writable for + `<style>` injection. +- **Boundary crossings**: The one real boundary crossing is + *parent document → injected `<object>` nested browsing context*. + Because the document is `about:blank`, the child inherits the parent origin + and the library reads `object.contentDocument` and attaches a listener to + `contentDocument.defaultView` directly. This is same-origin by design; it + would become a security boundary violation only if the `data` URL were ever + made caller-controllable or cross-origin. + +## Sensitive Data Paths + +| Data Type | Source | Destination | Protection | +|---|---|---|---| +| Element geometry (`offsetWidth`, `offsetHeight`, computed `position`/`top`/`right`/`bottom`/`left`) | Browser layout engine, via `getComputedStyle` in `scroll.js:storeStartSize` and `object.js` | In-memory element state (`state-handler.js`) and interpolated into generated CSS pixel strings | Numeric values only; never persisted or transmitted | +| Detector element IDs | `id-generator.js` (monotonic counter) | Element state and injected `<style>` element `id` | Sequential integers; not derived from and not correlatable to customer data | +| DOM element references | Caller (`react-sizeme` → `AdjustableDash.tsx`) | `reporter.warn`/`reporter.log` → browser console when style conflicts or debug mode are active | Console-only, client-side; could surface page structure in user-collected logs. No app data, credentials, or PII. | + +No secrets, tokens, credentials, PII, or customer app content pass through this +library. + +## Privileged Actions + +| Action | Location | Guard | +|---|---|---| +| Create and append a nested browsing context (`<object type="text/html" data="about:blank">`) into a caller element | `src/detection-strategy/object.js:injectObject` (inner `mutateDom`) | URL is the hardcoded literal `"about:blank"`; node is marked `tabIndex = -1` and `aria-hidden="true"`; IE ordering handled via `browserDetector.isIE()` | +| Register a listener on another document's window | `src/detection-strategy/object.js:addListener` (`object.contentDocument.defaultView.addEventListener("resize", listenerProxy)`) | Same-origin `about:blank` child only; removal depends on `object.js:uninstall` being called | +| Inject a `<style>` element into `document.head` | `src/detection-strategy/scroll.js:injectScrollStyle` (inner `injectStyle`) | Written via `textContent` (Trusted Types patch, not `innerHTML`); guarded by a `getElementById(styleId)` idempotency check; rule text built only from hardcoded constants and the fixed `detectionContainerClass` | +| Bulk `style.cssText` assignment on injected and host elements | `src/detection-strategy/scroll.js:getScrollbarSizes`, `:storeStartSize`, `:initListeners`; `src/detection-strategy/object.js:injectObject` | `buildCssTextString` joins hardcoded rule literals and numeric pixel values; no caller strings interpolated | +| Mutate the host element's inline position styles | `src/detection-strategy/object.js:alterPositionStyles`, `src/detection-strategy/scroll.js:alterPositionStyles` (`element.style.setProperty`) | Applied only when computed `position` is `static`; `!important` applied only when `options.important` is set; conflicts reported via `reporter.warn` | +| Schedule recurring timers | `src/detection-strategy/object.js:onObjectLoad` → `getDocument` (`setTimeout(..., 100)` poll for `contentDocument`) | Timer id stored in element state and cleared on re-entry and in `object.js:uninstall` | +| Remove injected nodes and all listeners | `src/element-resize-detector.js:uninstall` → `listener-handler.js:removeAllListeners`, strategy `uninstall` | Element state existence checked; correctness of full teardown is a priority review area | + +## Priority Review Areas + +1. **`<object>` injection in `src/detection-strategy/object.js` (highest + value).** This is the only place the library creates a new browsing context. + `data` is the hardcoded `"about:blank"` today, and because that inherits the + embedding origin, anything that made the URL caller-influenceable — an + option, an attribute read off the target element, an app-config value — + would turn a layout utility into a same-origin script-injection sink. + Confirm the literal is unreachable from configuration and that the injected + node cannot be re-pointed after insertion. +2. **Trusted Types patch integrity and `src`/`dist` parity.** The entire reason + this fork exists is `scroll.js:injectScrollStyle` using `textContent` + instead of `innerHTML`, and commit `4a11fdb` exists because `dist/` once + shipped without that fix. Verify the patch is present in `src/` **and** in + both `dist/` bundles, and treat any reintroduction of `innerHTML` in + `src/detection-strategy/` as a release blocker. The residual static-string + `innerHTML` in `src/browser-detector.js` should be confirmed unreachable + with non-literal data (and is a candidate for removal, since the IE probe is + dead code in supported browsers). +3. **Style and attribute construction across both strategies.** Every generated + CSS string flows through `buildCssTextString` into `style.cssText`, and CSS + rule text in `injectScrollStyle` interpolates a class name. These are the + library's string-building hot spots. Confirm all inputs remain hardcoded + literals or layout-engine numbers, and that `detectionContainerClass` is not + promoted to a caller option — a caller-supplied class name would become a + CSS-injection sink inside a live `<style>` element. +4. **Listener and detached-DOM leaks (client-side DoS).** Cross-document + listeners on `object.contentDocument.defaultView`, the + `checkForObjectDocumentTimeoutId` polling loop, scroll/animationstart + handlers on injected nodes, and the element→state map in + `state-handler.js` all persist until `uninstall` runs. AppSheet mounts and + unmounts dashboards repeatedly within a single session, so incomplete + teardown accumulates detached DOM and listeners and degrades or hangs the + tab. Verify `react-sizeme`'s unmount path actually reaches + `erd.uninstall`. +5. **Host-page layout mutation and UI redress.** The library forcibly sets + `position: relative` (optionally `!important`) on observed elements and + inserts absolutely-positioned hidden containers, including one temporarily + inserted at `document.body.firstChild` in `scroll.js:getScrollbarSizes`. + Review for overlay/clickjacking potential and for interference with + AppSheet's own z-index and modal stacking on customer app pages. +6. **Caller-supplied hooks (`idHandler`, `reporter`, `batchProcessor`).** These + options let arbitrary functions execute inside the library's DOM-mutating + code paths with full page privileges. Confirm AppSheet (via `react-sizeme`) + supplies only defaults, and that none of these can be reached from + app-author-controlled configuration. + +## Out of Scope + +Teams can fill out this section with any known filed vulnerability bugs that +they've determined should be out of scope from future Fortify bug filing.