admin/raven
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Security review pass 4: report rendering

Browse Source 
F19 (critical, confirmed on the local server): report template import has no
role check, so any login, customer contacts included, can add a template whose
JavaScript runs in the server's browser. From code: F20 (templates get the
renderer's token, SuperUser for customer notification reports, and unrestricted
network access), F21 (download file names from the URL not checked; Windows),
F22 (minor job and template gaps). Review only, no code changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
John Cardinal
2026-10-23 20:04:23 -07:00
parent 07c10f6542
commit 21c55e8d01
1 changed files with 68 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

70
devdocs/modernization/SECURITY-REVIEW.md
View File

@@ -14,8 +14,8 @@ keep it that way while findings are open.
| 1 | Threat model and attack-surface map | Done 2026-10-23 |
| 2 | Unauthenticated routes, login, tokens, password reset, 2FA | Done 2026-10-23 |
| 3 | Customer contact users: what they can reach and read | Done 2026-10-23 |
| 4 | Report rendering (templates, the server-side browser, report data and download routes) | Next |
| 5 | File handling: attachments, uploads, exports, logs, backups, restore, translations, logo | |
| 4 | Report rendering (templates, the server-side browser, report data and download routes) | Done 2026-10-23 |
| 5 | File handling: attachments, uploads, exports, logs, backups, restore, translations, logo | Next |
| 6 | Raw SQL: data lists, saved filters, search, KPI, schedule, pick lists | |
| 7 | Staff roles: the authenticated routes without a role check in the action (list below) | |
| 8 | Email and notifications (`direct-smtp`, templates, delivery) | |
@@ -188,12 +188,15 @@ Status: *candidate* (seen in code, not yet confirmed), *confirmed*, *fixed in ca
|---|---|---|---|
| F5 | **Critical** | **confirmed** | 2FA login step accepts an empty or missing temp token: the password is skipped for users with 2FA on |
| F1 | Critical (installs that kept it) | **confirmed** | Placeholder JWT secret in the LAN/IIS and Linux server config templates |
| F19 | Critical | **confirmed** (import) | Any logged-in user, customer contacts included, can import report templates, whose JavaScript runs in the server's browser |
| F21 | Critical | candidate | Download file names from the URL are not checked: `report/download` and `export/download` may serve files outside the temp folder on Windows |
| F18 | Critical | candidate | Pick-list search text is put into SQL without escaping; any logged-in user can reach it |
| F13 | High | **confirmed** | Customer contacts can open any customer's work order (customer view, customer report, attachments) by id |
| F14 | High | **confirmed** | Customer contacts can read, and by code change, delete and create, any customer's service requests |
| F15 | High | **confirmed** | Customer contacts can list every customer (and vendors, users, parts...) with any pick-list columns, phones and emails included |
| F16 | High | **confirmed** | `schedule/svc` and `schedule/user` have no role check: anyone logged in sees the whole service schedule |
| F17 | High | candidate | Notification subscriptions accept any `UserId`: a contact could subscribe on a staff user's behalf with delivery to its own email |
| F20 | High | candidate | Report templates receive the renderer's token (SuperUser for customer notification reports) and the report browser can reach any network address |
| F3 | High | candidate | Any logged-in user, customer contacts included, can send arbitrary email through the company's SMTP server |
| F6 | Medium | confirmed | No lockout or rate limit on login or the 2FA step; no password rules |
| F9 | Medium | confirmed | Default SuperUser password, and an anonymous route that says whether it is still set |
@@ -204,6 +207,7 @@ Status: *candidate* (seen in code, not yet confirmed), *confirmed*, *fixed in ca
| F10 | Low | confirmed | Anyone can record acceptance of the licence agreement (`license/lc`) |
| F11 | Low | confirmed | API root page shows licensee, version, licence terms (and DB ID when unlicensed) to anyone |
| F12 | Low | confirmed | Password hashing: PBKDF2-SHA512 with 10,000 iterations |
| F22 | Low | confirmed | Minor report and job route gaps: template export by any download key, job status, logs and cancel by GUID without an owner check, browser without sandbox |
**Release decisions.** John, 2026-10-23: **F13 to F17 are urgent and are fixed in 8.3.0**, as
"next release" cases (case numbers to come). Hosted and self-hosted perpetual installations
@@ -416,6 +420,67 @@ texts as SQL parameters (preferred) or escape them as the data-list builder does
rest of the pick-list builder in pass 6.
### F19 Report template import open to every login (pass 4, confirmed)
`POST report/upload` (`ReportController.UploadAsync`) has `[Authorize]` only, and
`ReportBiz.ImportAsync` checks nothing either; creating or editing a template through the
normal routes needs `Change` on `Report` (BizAdmin, BizAdminRestricted). A template holds
JavaScript (`JsPrerender`, `JsHelpers`) and a Handlebars template, which `ReportBiz` loads into
the headless browser on the server at render time. **Confirmed**: the seeded contact
`Customer` imported a copy of a stock template (202, new report 104, deleted afterwards).
Consequences (from code; the rendering side was deliberately not tested):
- `report/render-job` requires read rights on `Report` (everyone) and on the template's
record type, which the uploader chooses (a contact can read CSRs), so any login can run
its own JavaScript in the server's browser: it can reach the local API and any address the
server can reach (internal network, cloud metadata endpoints) and put the results into the
PDF it downloads.
- An imported template also appears in staff report lists; when staff or SuperUser render it,
it runs with their token (see F20).
Fix: in `UploadAsync`, require `HasCreateRole(Report)` like `POST report`; F20's fixes limit
what a template can do even for legitimate authors.
### F20 What a report template can reach (pass 4, candidate)
The page gets the renderer's own bearer token in `AYMETA.ayClientMetaData.Authorization` (for
`ayGetFromAPI`/`ayPostToAPI`/`ayPutToAPI`), and nothing restricts the page's network access
(no request interception in `ReportBiz`). So a template's script can send the token of
whoever renders it to any outside address; that token is the user's current session token,
usable from anywhere. Customer notifications with an attached report
(`CoreJobCustomerNotify`) render with `AuthController.GenRpt`: a SuperUser (id 1) token with an
`rpl` claim, accepted from 127.0.0.1 for the render timeout. A template author (including the
"restricted" `BizAdminRestricted` role) can therefore act as SuperUser, or as any colleague who
runs the report. Fix options: intercept page requests (PuppeteerSharp
`SetRequestInterceptionAsync`) and allow only the local API URL and `data:`/`blob:`; render
notification reports as a dedicated low-rights identity rather than SuperUser; consider a
short-lived token limited to read routes for template API calls.
### F21 File names from the URL in download routes (pass 4, candidate, code reading)
`report/download/{fileName}` and `export/download/{fileName}` accept any user's download key
and serve `FileUtil.GetFullPathForTemporaryFile(fileName)`, which is
`Path.Combine(TemporaryFilesFolder, fileName)` with no check. On Windows, a backslash in the
route value is not a separator for routing but is one for `Path.Combine`, and an absolute
name replaces the folder entirely; the result is passed to `PhysicalFile`. Any login
(customer contacts included) could then read files outside the temp folder, such as
`config.json` (database password, JWT secret). Linux is likely not affected (`/` stays encoded
as `%2F` in route values); IIS request filtering may block some forms. Not tested live by
choice. The same unchecked pattern is in `log-file/{logname}`, `log-file/download/{logname}`
and the backup download and delete routes (ops roles; pass 5). Fix: accept only a bare file
name (`Path.GetFileName(name) == name`, no `..`), and check the resolved full path is inside
the folder, in one helper used by every such route.
### F22 Minor report and job gaps (pass 4)
`report/export/{id}` gives any report template to any valid download key (templates are not
secret, but staff-only templates go to customer contacts). `job-operations/status|progress|
logs/{gid}` and `report/request-cancel/{gid}` check no owner; job ids are random GUIDs, so
this needs a leaked id. The browser runs with `--no-sandbox` (Chrome's own sandbox off), so a
browser bug reached through a template is not contained; Linux runs as `www-data`, Windows LAN
as the logged-in user. Fix: owner check on job routes; consider enabling the sandbox where
the platform allows.
### F2 Windows folder permissions (from the 8.3.0 installer test)
`lan.iss` and `standalone.iss` set `Permissions: users-modify` on `{app}` (the program
@@ -489,3 +554,4 @@ roles that should not see the schedule (pass 7).
| 2026-10-23 | 1 Threat model and map | Scope agreed with John. Middleware, JWT validation, role model, download keys, CORS and Swagger read. Route map generated (460 actions; script kept in `security/routemap.py`). No route lacks authentication by accident. 53 logged-in routes have no role check in the action: 3 resolved (erase data, trial seed: OK; direct SMTP: F3), the rest assigned to later passes. Local-request exception not reachable through the hosted nginx. Findings F1 to F4 recorded as candidates |
| 2026-10-23 | 2 Login, tokens, reset, 2FA, anonymous routes | Read `AuthController`, the token middleware, `Hasher`, download-key check, password reset, TOTP setup, and every anonymous route (`notify/hello`, `license/lc`, `translation/subset`, `logo`, API root, health). Tested on the local dev server with the seeded `Inventory` user: **F5 critical, confirmed** (2FA step without password); **F1 confirmed** (current token rebuilt from the secret in 6 tries); F8 confirmed. New: F6, F7, F9 to F12. OK: reset codes (32 random characters, 48 h, cleared on use, same reply for unknown codes), login replies do not reveal whether a login exists, roles read from the database on every request, deactivation ends the session, TOTP secret 160 bits. Leads for pass 4: `AuthController.GenRpt` issues SuperUser (id 1) tokens with an `rpl` claim for report rendering; pass 5: the logo is served with the content type given at upload |
| 2026-10-23 | 3 Customer contacts | Mapped the `Customer` role in `BizRoles.cs` and every customer-reachable route. Tested on the local dev server as seeded contact `Customer` (customer 1), read-only: F13, F14 (read), F15, F16 confirmed. F17 and F18 from code (F17 not tested because it sends email; F18 not tested by choice). OK: data lists (server sets the customer filter), KPIs (no customer roles), saved filters and notification subscription reads (scoped to the user), memos (no customer access) |
| 2026-10-23 | 4 Report rendering | Read `ReportController`, `ReportBiz` (render path, data, browser launch), `ay-report.js` helpers, `CoreJobCustomerNotify` and `AuthController.GenRpt`. Tested on the local dev server: **F19 confirmed** (contact imported a stock template copy, deleted afterwards). F20, F21 and F22 from code; rendering of a crafted template and the download path were deliberately not tested. OK: `report/data` and `render-job` check read rights on the record type; template API calls use the renderer's own token (except notification reports, F20); report output files have random names. Pass 5 picks up F21's other routes |