36 KiB
18 — Red-Team Security Review (2026-07-10)
Adversarial review of the SiMS Next build by five parallel reviewers, each reading the
code and firing proof-of-concept requests at the live dev server (:5181). Every finding
is labelled CONFIRMED (proven with a PoC) or THEORETICAL. Nothing was fixed —
this is assessment; remediation is the founder's call (see plan at the end).
The one-sentence verdict
The store-server authenticates requests but does not authorize them and does not
validate or anchor the business data in them — so any logged-in cashier (the lowest
role) can, with curl, rewrite prices, evade GST, edit the item master, and forge the
audit trail. The React app enforces all these rules; the server trusts the client to have
done so. Nothing is shipped to a real store yet, so this is the right moment to find it.
Why "the server re-verifies every bill to the paisa" (R5) gave false confidence: it proves the client's total matches the client's own line prices — internal consistency, not authenticity against the item master. The supervisor gate we verified in the browser is real in the UI and unenforced on the server.
Remediation status — P0 CLOSED (2026-07-10)
The P0 workstreams below are REMEDIATED: the trust boundary now lives inside the store-server (item-master anchoring, per-route authorization, input validation, bound approval tokens). The dev DB was reset and every PoC re-fired against the fresh server; each that used to succeed now fails closed. A real-browser E2E confirms the honest counter flow still works. P1 and P2 remain open (see the plan at the end).
| PoC | Before (assessment) | After (2026-07-10) | Evidence |
|---|---|---|---|
C1 cashier → POST /items (edit master) |
200, item written | 403 E-6105 | authz middleware; a cashier may still POST /items a draft (200) — now with a server-forced tax class + price cap + DRAFT_SALE audit (see the 2026-07-13 follow-up below) |
C1 cashier → GET /audit |
200, full log | 403 E-6105 | AUDIT_VIEW gate |
C1 cashier → GET /purchases/last-costs, /purchases |
200, supplier costs | 403 E-6105 | PURCHASE_ENTRY/REPORT_VIEW gate |
| C2 cashier bills a ₹275 item at ₹1 | committed, no audit | 403 E-1302 (deviation needs approval) | server anchors master price; nothing committed |
C3 12% item sent as taxClassCode:ZERO |
committed, ₹0 GST | committed at GST12 (CGST₹14.73+SGST₹14.73 on the ₹275 bill) | tax class read from master, request ignored |
C4 cashier POST /purchases w/ newSalePricePaise |
master price rewritten | 403 E-6105 (route denied); a PURCHASE_ENTRY-only role receives stock but the price change is ignored (needs MASTER_EDIT) |
authz + MASTER_EDIT split |
| H4 bound token for item A reattached to item B | audit falsely records the approver | 403 E-6108 (bound mismatch); same token used correctly on item A → 200 | token binds item/kind/before/after |
H5 itemId:"DOES-NOT-EXIST" |
phantom bill + stock | 400 E-1105 | item verified vs (id, tenant) |
H6 businessDate 2050-07-10 / not-a-date |
postdated bill / garbage series | 400 E-1106 | ISO + windowed validation |
| H7 owner/supervisor self-authorised override | no audit row at all | audited from the session identity (selfAuthorized:true) |
server auto-detects + audits every deviation |
M6 /items?limit=-1 |
whole table | clamped (≤ 1 row) | limit clamped [1,MAX], offset ≥ 0 |
| M7 wrong types | 500 + stack + install path | 400 {error,code} generic, no stack |
hand-rolled validators + 4-arg error handler |
| M8 negative/mismatched payment legs, fractional qty | slipped through | 400 (leg > 0, allowlisted mode, sums to payable; unit-decimals enforced) | payment + qty guards |
| M12 giant line count | unbounded | ≤ 500 lines (400 over cap) | assertArray cap |
M14 X-Powered-By: Express banner |
present | removed | app.disable('x-powered-by') |
Real-browser E2E (fresh DB, Chrome): Ramesh 4728 → normal Surf Excel bill ST1C2/26-000004
commits at ₹145; Ramesh F3 rate ₹145→₹140 → Suresh 8265 → bound-token commit
…000005, audited approvedBy=Suresh; Thomas 9174 owner self-price-change → commit
…000006, audited approvedBy=Thomas, selfAuthorized:true. Tests 207 green; typecheck +
all three app builds clean.
HIGH follow-up — cashier draft quick-add path CLOSED (2026-07-13)
The independent verifier found that the C1 remediation left one HIGH hole inside the
quick-add convenience it deliberately kept open (an unknown-barcode item must not stop the
queue): a cashier (BILL_CREATE, no MASTER_EDIT) could POST /items with status:'draft',
choosing an arbitrary salePricePaise AND an arbitrary taxClassCode, then bill it.
Because commitBill anchors each line to the just-authored master, this reproduced C2
(arbitrary price) and C3 (GST evasion — a taxable good self-added as ZERO, sold at ₹0 GST)
with no deviation/override audit row — the only trace was ITEM_CREATE. Proven live: draft
"Redteam Whiskey" ZERO tax @ ₹2000 → billed with ₹0 GST.
Remediated (2026-07-13) — the quick-add convenience is kept, the fraud is removed:
| Vector | Fix | Evidence (re-fired live as cashier u1/4728, fresh DB :5181) |
|---|---|---|
C3 via draft — self-add taxable good as ZERO |
Server forces the tax class on a non-MASTER_EDIT caller's draft to the store default (catalog.defaultTaxClass, resolved system→tenant, default GST18); the client's taxClassCode is ignored. A MASTER_EDIT role (manager/owner) still sets it. |
draft ZERO→server assigned GST18; billed the draft → CGST+SGST = 30508 paise (was ₹0) |
| C2 via draft — self-price an expensive unknown | A non-MASTER_EDIT draft's salePricePaise is capped (catalog.draftMaxPricePaise, default ₹5,000 = 500000); over-cap is denied with a clear message. A MASTER_EDIT role may exceed it. |
draft @ ₹9,999 → 400 E-1108 "call a supervisor"; owner @ ₹9,999 → 200 |
| Invisible draft sale — self-authored item sold with no trail | Every bill line whose master row is still status:'draft' writes a DRAFT_SALE audit row (entity bill, {itemId, name, pricePaise, docNo}) in the commit transaction. Back office already shows draft status on Items. |
selling the two drafts wrote 2 DRAFT_SALE rows; an active-item sale writes none |
| Cross-counter bill read (residual LOW/MED) | GET /bills for a caller without REPORT_VIEW is pinned server-side to the session user id — a cashier sees only their own bills; a REPORT_VIEW/manager keeps the full view. |
Ramesh → only u1 rows; Divya → only u4 rows; owner → all cashiers |
Regression re-check (same run): C2 existing-item ₹275→₹1 no-approval → 403 E-1302;
C3 existing-item ZERO@master → committed taxed at GST12 (CGST+SGST 2946 paise);
C1 cashier POST /items status:'active' → 403 E-6105, cashier GET /audit → 403.
Legit quick-add still works end-to-end (unknown barcode → draft @ ₹30 → 2× bill ST1C2/26-000004
₹60). 220 tests green (9 new: draft tax forced, over-cap E-1108, DRAFT_SALE audit,
cashier /bills scoped, owner exceptions); typecheck + all three app builds clean. Back office
must complete each draft's real HSN/tax class later (the forced default is a safe placeholder,
not a substitute for classification).
P1 edge & transport hardening — CLOSED (2026-07-13)
The P1 "lock down the edges + TLS" workstream is REMEDIATED: H1, M1, M2, M4, M13, M14
are closed and M15 is noted (closed once prod is HTTPS). Server changes stayed in
server.ts + api.ts plus three pure, unit-tested modules (print-guard.ts, rate-limit.ts,
session-policy.ts); client edits were limited to logout wiring + the POS post-login bootstrap
re-fetch. No new runtime dep except helmet (justified). All PoCs were re-fired live against
:5181 (the HTTP dev instance, unchanged); the session-TTL and rate-limit PoCs ran against a
short-lived isolated instance (:5182, own data dir) so the dev DB and :5181 stayed clean.
(M5 — tenant-scoping commitBill's store/counter lookup — is owned by the money-logic
workstream and is NOT part of this pass.)
| # | Before | After (2026-07-13) | Evidence (live) |
|---|---|---|---|
H1 SSRF via POST /api/print |
authed cashier opened a socket to ANY host:port + bytes (internal port-scan / egress) | destination constrained: LAN-only IP (127/8,10/8,172.16/12,192.168/16; link-local/public rejected), port allowlist {9100,9101,9102} (+SIMS_PRINTER_PORTS), ≤64 KB payload; the host is resolved and every resolved IP checked, then we connect to the resolved IP (no DNS-rebind). Now an authenticated route inside apiRouter |
authed → 8.8.8.8:80 400 E-2202, 8.8.8.8:9100 400 E-2203 (no socket opened), 127.0.0.1:9100 502 ECONNREFUSED (guard passed → socket attempted); unauth → 401 |
M1 /bootstrap leaks roster roles + GSTIN pre-auth |
the unauthenticated payload carried each user's role + the tenant GSTIN (targeting data) | the pre-auth payload drops role + gstin (keeps id+name for PIN login); the SAME endpoint restores both when called WITH a session — the POS re-fetches after login, back office is always authed | unauth /api/bootstrap → store has no gstin, users are {id,name} only (grep for gstin/role: none); authed → gstin 32ABCDE1234F1Z5 + roles present |
| M2 verify-pin unauth PIN oracle / token mint | callable with NO session — a raw PIN-guessing + token-minting endpoint | now behind requireAuth (a logged-in caller) and rate-limited; the approver PIN still rides in the body | unauth POST /auth/verify-pin → 401 E-6103; authed cashier caller + Suresh/8265 → 200 + bound approvalId; wrong approver PIN → still 401 |
| M4 no session expiry / logout / revocation | a token was valid until the process restarted | sessions carry created + last-seen; requireAuth evicts past a 12 h absolute / 30 m idle TTL (→ 401 E-6110); POST /api/auth/logout deletes the session (POS "Lock" + back-office "Logout" call it); global + per-IP /auth/* rate limit (20/min/IP, 200/min global, env-tunable) layered on the per-user lockout |
logout: /items 200 → logout 200 → /items 401; idle (1 s-TTL instance): after 1.4 s → 401 E-6110; rate limit (cap 3): attempts 1-3 = 401, 4-5 = 429 |
| M13 plaintext HTTP | bearer token rides the LAN in cleartext | HTTPS available: SIMS_TLS_CERT+SIMS_TLS_KEY set ⇒ https.createServer; unset ⇒ HTTP dev (unchanged). Dev-cert one-liner + "prod MUST supply certs" documented in server.ts. HSTS + upgrade-insecure-requests emitted only under TLS (so dev HTTP is never told to upgrade to a port that serves no HTTPS) |
dev :5181 stays HTTP (verification/browser flow intact); the TLS path is code-reviewed, not forced on dev |
M14 no security headers / X-Powered-By |
no CSP, no nosniff/frame/referrer headers | helmet on with a CSP tuned for the SPAs ('self' scripts + 'unsafe-inline' styles for React inline styles + blob:/data: for the invoice/label tabs + data: fonts; the blob tabs' only inline script — window.print() — is allowed by a single 'unsafe-hashes' sha256, NOT by opening script-src to 'unsafe-inline'); X-Frame-Options: DENY (helmet only ships SAMEORIGIN) + frame-ancestors 'none'; nosniff; Referrer-Policy: no-referrer; x-powered-by already gone |
curl -i /api/health shows the full header set, no X-Powered-By, no HSTS on HTTP; back office + POS both load in a real browser with ZERO CSP console violations, and the window.print() inline handler is not blocked |
| M15 SW cache-first over HTTP → MITM poisoning | THEORETICAL, needs LAN MITM | noted — closes automatically once prod serves HTTPS (M13); no code change needed for the dev slice | n/a (documentation) |
Tests: two new files — edge-guards.test.ts (pure: LAN/port/byte print validator,
sliding-window rate limiter, session-TTL check) and edge.test.ts (HTTP against a mounted
apiRouter: bootstrap minimize, verify-pin auth, TTL eviction, logout, rate-limit 429, print
allowlist). 245 tests green (was 220), typecheck clean on root + all three apps, all three
apps build, store-server rebuilt + restarted on :5181 (/api/health ok). Browser check: back
office login + POS login screens render with no CSP violations; the POS login tiles now show
no role subtitle (the pre-auth roster no longer carries roles — the expected M1 effect).
Residue: the live PoCs added one benign, unused override_approval row to the :5181 dev DB
(a verify-pin mint); no bills/items/stock were written. No git commits.
P2 data-integrity & crypto — CLOSED (2026-07-13)
The P2 "harden-before-cloud" data-integrity slice is REMEDIATED: H2, M3, M5, M9, M10 are
closed (M11 client-PII and M14 helmet closed earlier). Only H3 (encryption at rest) remains,
deliberately owned by a separate agent next. Changes were confined to apps/store-server/src/{db.ts, repos.ts, api.ts, server.ts}, packages/auth/src/pin.ts, packages/domain/src/doc-series.ts, plus
dev-start.mjs + .env.example; the transport/helmet/TLS and requireAuth/rate-limit/session code
from the P1 pass was left intact (only writeAudit and the commit/series paths were touched).
| # | What changed | Evidence (live on the reset :5181, SIMS_SEED_DEMO=1) |
|---|---|---|
| H2 audit hash chain | audit_log gained prev_hash+entry_hash (CREATE TABLE + PRAGMA-guarded migrate). writeAudit links each row into the tenant's chain inside the existing commit transactions (entry_hash = sha256(canonical(prev_hash, tenant_id, at_wall, user_id, action, entity, entity_id, before_json, after_json)), genesis '', insertion order = SQLite rowid under the single-writer model). verifyAuditChain(db, tenant) recomputes and returns {ok, brokenAtId?}; GET /api/audit/verify (AUDIT_VIEW) exposes it. Tamper-EVIDENCE, not prevention — the file is still writable (H3). |
Owner (thomas/9174) GET /api/audit/verify after a real bill → {ok:true}; then a direct better-sqlite3 UPDATE audit_log SET after_json=… on one row → verify → {ok:false, brokenAtId:<that row>} (brokenAtId matched the tampered row exactly). |
| M3 PIN pepper | pin.ts HMAC-SHA256s the secret with SIMS_PIN_PEPPER before scrypt in both hashPin/verifyPin; unset ⇒ raw-secret fallback (byte-identical to pre-M3) + one-time warning. .env.example documents it; enabling requires PIN re-enrolment. |
Unit tests: round-trip with a pepper; a peppered hash fails to verify once the pepper is gone (a DB-only theft is uncrackable) and again with a different pepper (rotation invalidates); unset path still verifies. Dev logins on :5181 unchanged (pepper unset). |
| M5 tenant-scope commit | store AND counter selected WHERE id=? AND tenant_id=?; unknown-for-tenant → 400 E-1110 before any money logic (kills the undefined-deref 500). Both /bills and /bills/offline covered. |
POST /api/bills with storeId:"no-such-store" → 400 {"code":"E-1110"} (not 500). Unit: a store/counter belonging to another tenant is rejected; honest same-tenant commit still bills. |
| M9 4-digit FY | seriesPrefix embeds the 4-digit FY start year; the counter↔FY / is dropped so the doc no fits the GST 16-char rule at 6-digit sequences (ST1C22026-000123). Old ST1C2/26-… collided a century apart. |
Fresh bill on :5181 → doc no ST1C22026-000001 (length 16, matches ^ST1C22026-\d{6}$). Unit: seriesPrefix('ST1','C2','2026-27')≠…'2126-27', both ≤16 at seq 999999. |
| M10 demo-seed gate | seedIfEmpty/ensureBatchTrackingSeed run only when SIMS_SEED_DEMO=1; dev npm start → dev-start.mjs sets it. |
Isolated instance, flag unset → healthy but 0 tenants, thomas login 401. :5181 with the flag set → seeds MegaMart + 4 users as before. |
Tests: 260 green (was 245): new audit-chain.test.ts (5: chain continuity + UPDATE/DELETE/INSERT
detection + per-tenant scope), tenant-scope.test.ts (4: nonexistent/cross-tenant store/counter reject,
honest commit), a /audit/verify HTTP case in edge.test.ts, 4 PIN-pepper cases in auth.test.ts, and
a 4-digit-FY collision + 16-char case in domain.test.ts. Typecheck clean on root + all three apps; all
three apps build; the dev DB was reset and the store-server restarted on :5181 with SIMS_SEED_DEMO=1
(/api/health ok, seeded, audit/verify ok on the clean chain). H3 (encryption at rest) is the only
open P2 item. No git commits.
H3 — encryption at rest — CLOSED (2026-07-13)
The last open finding is REMEDIATED: the store DB is now full-DB encrypted at rest with
SQLCipher, which closes H3, the F5 at-rest concern, and the M11 residual (the offline
queue's lines/amounts sitting in plaintext until drained — the server tier they drain into is now
encrypted). better-sqlite3 was swapped for the API-compatible drop-in
better-sqlite3-multiple-ciphers (same API + pragma('key=…'); it ships its own
DefinitelyTyped-derived declarations, so DB and every call type unchanged — no shim, no loosened
strictness). The change was confined to apps/store-server/src/db.ts, package.json,
build-server.mjs and .env.example; none of the earlier hardening (pepper, hash chain,
tenant-scope, seed gate, TLS/helmet, print allowlist) was touched.
Path taken: full-DB encryption. The native package installed from a prebuilt binary (no node-gyp build) on this Windows / Node 22 box, so the app-level field-encryption fallback was not needed.
| Vector | Fix | Evidence (live on the reset, keyed :5181) |
|---|---|---|
Whole file readable if copied — a stolen/snooped sims.db (+ WAL) yielded phones/GSTINs (DPDP), costs & margins, full sales history, party ledgers, PIN salts/hashes |
In openDb(), when SIMS_DB_KEY is set the DB is keyed (cipher='sqlcipher', key=…) before any read, so every page — main db and the WAL/SHM sidecars — is ciphertext. The key lives ONLY in the server env (a machine-scoped DPAPI/keychain secret), never in the repo or the DB; single quotes in it are escaped so an arbitrary passphrase is a safe SQL literal. |
head -c 16 data/sims.db → 28 85 83 94 c9 b5 4f 93 … (random), not 53 51 4c 69 … ("SQLite format 3\0"). Grep of sims.db / -wal / -shm for MegaMart, Lakshmi, the phone 9840012345, and the tenant GSTIN 32ABCDE1234F1Z5 → none found in cleartext. |
| A stolen copy must be unreadable without the key | Wrong/absent key fails closed | Opening the live data/sims.db without the key → file is not a database; with a wrong key → file is not a database; with the correct key → opens (tenant count 1, parties ["Lakshmi","Joseph",…] decrypt). |
| The app must still fully work through the cipher | The keyed DB is transparent to the app | Cashier Ramesh (u1/4728) PIN login 200; GET /items?all=1 200 (11 items decrypt); POST /bills 200 → ST1C22026-000001 (payable ₹5.00); owner Thomas (u3/9174) GET /audit/verify → {ok:true} on a chain written through the encrypted DB. /api/health ok. |
| Dev fallback unchanged | Key UNSET ⇒ open unencrypted exactly as before + a one-time warning that prod MUST set it (mirrors the SIMS_PIN_PEPPER pattern) |
Unit test: key unset ⇒ file is plaintext (SQLite format 3\0) and the phone is present in cleartext — a positive control proving the byte-scan genuinely detects plaintext. The one-time boot warning fires only when unset; with the dev key set on :5181 it does not. |
Tests: 265 green (was 260; +5 in new encryption.test.ts: a keyed file has no SQLite magic +
no cleartext PII across db/WAL, the keyed data round-trips, opening without / with the wrong key throws,
and the plaintext-fallback positive control). Typecheck clean on root + store-server; all three apps
build (esbuild externals the ciphers native module; vite unaffected). The dev DB was reset (data/
deleted — it held red-team junk + a deliberately-tampered audit row) and the store-server restarted on
:5181 with a dev SIMS_DB_KEY, so encryption is live and proven end-to-end while the :5181 flow
still works.
A production deploy still MUST: (1) supply SIMS_DB_KEY as a machine-scoped secret (Windows
DPAPI or an OS keychain) — never a plaintext file beside the DB, never in the repo; (2) migrate any
existing plaintext sims.db once, offline (ATTACH … KEY …; SELECT sqlcipher_export('enc'); DETACH
— documented in openDb() and .env.example) before the first encrypted boot; a fresh/empty DB is
created encrypted automatically; (3) treat key rotation as a re-key (PRAGMA rekey) or an
export/reimport, and back up the key with the DB (an unrecoverable key = an unrecoverable DB); (4)
still tighten the data-dir ACLs as defence-in-depth. No git commits.
Clean bill of health (verified defended — do not "fix")
| Area | Verdict |
|---|---|
| SQL injection | None — every query is a bound better-sqlite3 prepared statement; DROP TABLE payloads inert |
| XSS / HTML injection | None — every invoice/label/receipt sink escapes; React auto-escaping intact; zero dangerouslySetInnerHTML |
| Dependency supply chain | npm audit 0 vulnerabilities across 542 deps; lockfile committed |
| CSRF / CORS | Not vulnerable — bearer-token-in-header design (not cookies) defends both |
| doc-series under load | Crash-safe & gap-free by transaction design (genuine strength, S7-proven) |
| Approval token entropy/TTL/single-use | Correct — 5-min TTL + atomic single-use enforced server-side |
| Engine numeric guards | Rejects negative/zero/NaN qty, negative price, >100% discount, payment mismatch, >1 MB body |
Findings — ranked
CRITICAL — exploitable now by any cashier session
| # | Finding | Evidence | Fix |
|---|---|---|---|
| C1 ✅ REMEDIATED | No server-side authorization. RBAC (permissions.ts can()/gateFor()) is called only in client code; api.ts never imports it. Every route past requireAuth checks only that a token exists. |
Cashier token created items, read full /audit, dumped supplier costs, listed all counters' bills — all 200. api.ts:138 is the lone gate. |
Per-route authorization middleware mapping route→ActionCode, can(session, action) server-side, deny by default. |
| C2 ✅ REMEDIATED | Bill line prices trusted from client; the override/discount gate is client-only and curl-bypassable with no audit. commitBill recomputes from input.lines and never reads item.sale_price_paise/mrp. |
Cashier sold a ₹270 item at ₹1 — committed, no override, no PIN, no audit row (repos.ts:223, compute.ts:41-61). |
Load each line's master price; reject any deviation unless a bound override token is redeemed; hard-block price > MRP. |
| C3 ✅ REMEDIATED | Client-chosen tax class → GST evasion. taxClassCode comes from the request; resolveTaxClass trusts it. |
A 12% item sent as ZERO committed with ₹0 GST (compute.ts:81). Scales to the whole ledger. |
Resolve tax class from the item master, never the request. |
| C4 ✅ REMEDIATED | Purchase posting rewrites master price/MRP with no role check. | Cashier posted a purchase with newSalePricePaise:99999; the item master changed (api.ts purchase route → repos-purchase.ts:99-106). |
Require PURCHASE_ENTRY; separate "receive stock" from "change master price". |
HIGH
| # | Finding | Evidence | Fix |
|---|---|---|---|
| H1 ✅ REMEDIATED (2026-07-13) | Authenticated SSRF via POST /api/print — arbitrary host:port + bytes; clean internal port-scan oracle. (Correctly auth-gated: unauth → 401.) |
Cashier token opened sockets to chosen host:port; closed→502, open→504 (server.ts:37-58). |
DONE: LAN-only IP + port allowlist ({9100,9101,9102}, SIMS_PRINTER_PORTS) + ≤64 KB cap; host resolved and each resolved IP LAN-checked, connect to the resolved IP (no rebind). print-guard.ts + the /print route in api.ts. See the P1 section. |
| H2 ✅ REMEDIATED (2026-07-13) | Audit log & "immutable" bills are tamper-evident by convention only — no hash chain, no signing; plain SQLite file. | Any file-level UPDATE/DELETE rewrites bills, deletes override evidence, forges approvals (repos.ts:431-443, schema). Undercuts the GST/MCA audit-trail claim. |
DONE: per-tenant hash chain on audit_log (prev_hash/entry_hash; entry_hash = sha256(canonical(prev_hash, tenant_id, at_wall, user_id, action, entity, entity_id, before_json, after_json))), written in-transaction by writeAudit; verifyAuditChain + GET /api/audit/verify (AUDIT_VIEW) recompute it. Any out-of-band UPDATE/DELETE/INSERT breaks the chain visibly. Tamper-EVIDENCE, not prevention — sealing the writable file is H3. See the P2 section. |
| H3 ✅ REMEDIATED (2026-07-13) | No encryption at rest — PIN hashes, phone numbers (DPDP), GSTINs, costs, full sales history in a plain sims.db (+ WAL). |
A copied file yields the whole business + every staff PIN hash (db.ts new Database(file)). |
DONE: full-DB SQLCipher via the drop-in better-sqlite3-multiple-ciphers; openDb() keys the DB from SIMS_DB_KEY (env-only, machine-scoped) before any read, so sims.db + WAL/SHM are ciphertext; key unset ⇒ plaintext dev fallback + one-time warning. Proven live: header not SQLite format 3, no cleartext PII in db/WAL, open-without-key fails, app (login+bill+audit/verify) works with the key. Prod must supply the key via DPAPI/keychain + migrate an existing plaintext DB once (sqlcipher_export). See the H3 section. |
| H4 ✅ REMEDIATED | Approval token not bound to the override it authorizes — redirectable. Token stores only approver+time+used, not item/kind/amount/cashier. | A real "Milk ₹50→45" approval was reattached to "Whiskey ₹2000→200"; audit falsely records the supervisor (repos.ts:184-190, 303-337). Audited before/after are client-supplied too. |
Bind item/kind/before/after (+cashier) into the token at mint; re-verify at redeem. |
| H5 ✅ REMEDIATED | Item never validated → phantom / cross-tenant lines. No SELECT item, no FK on stock_movement.item_id. |
itemId:"DOES-NOT-EXIST" committed a bill + phantom stock movement (repos.ts:192-343). |
Verify itemId belongs to tenant+store; add FK. |
| H6 ✅ REMEDIATED | Unvalidated businessDate → antedated/postdated GST invoices + garbage series. |
2050-07-10 postdated a bill; "not-a-date" produced doc_no ST1C2/N--000001 (repos.ts:239). |
Validate ISO date, reject NaN FY, bound to the open business day. |
| H7 ✅ REMEDIATED | Self-authorized overrides (owner/supervisor at POS) are never audited end-to-end. | gateFor==='allow' applies the edit with no override entry, so nothing reaches the server or the log (BillingScreen.tsx:431). The least-supervised roles leave no trail. |
Emit an audited (server-recomputed) override for every price/discount change, including self-authorized. |
MEDIUM
| # | Finding | Fix |
|---|---|---|
| M1 ✅ REMEDIATED (2026-07-13) | Unauthenticated /api/bootstrap leaks the full user roster (ids, names, roles, who has a PIN) + store GSTIN → targeted PIN attack. |
DONE: pre-auth payload drops role + gstin (keeps id+name for PIN login); both restored on the SAME endpoint when called WITH a session (POS re-fetches post-login). See the P1 section. |
| M2 ✅ REMEDIATED (2026-07-13) | /auth/verify-pin is an unauthenticated PIN oracle that also mints approval tokens and writes DB rows on each call. |
DONE: now behind requireAuth (a logged-in caller) + the /auth/* rate limiter; approver PIN still in the body. |
| M3 ✅ REMEDIATED (2026-07-13, pepper) | Weak PIN throttle — in-memory, per-user, no global/IP limit, resets on restart; 4-digit space; scrypt cost immaterial vs the tiny keyspace; no server-side pepper. | DONE (pepper): pin.ts HMAC-SHA256s the secret with a server-held pepper (SIMS_PIN_PEPPER) BEFORE scrypt, in both hashPin/verifyPin, so a stolen DB can't be brute-forced against the 10k PIN keyspace without the env-only pepper. Unset ⇒ raw-secret fallback (dev unchanged) + one-time warning. .env.example added; enabling requires re-enrolling PINs (dev re-seed does it). The IP/global /auth/* limiter already landed in M2/M4; longer approver PINs deferred (policy). See the P2 section. |
| M4 ✅ REMEDIATED (2026-07-13) | No session expiry, logout, or revocation — a leaked token is valid until the process restarts; a fired employee can't be cut off. | DONE: 12 h absolute + 30 m idle TTL evicted in requireAuth (→ 401 E-6110); POST /api/auth/logout deletes the session (POS Lock + BO Logout wired); global + per-IP /auth/* rate limit (rate-limit.ts, session-policy.ts). |
| M5 ✅ REMEDIATED (2026-07-13) | commitBill store/counter lookup not tenant-scoped → cross-tenant reference (latent, single-tenant dev DB) + a bad storeId 500s the handler. |
DONE: commitBill selects store AND counter WHERE id=? AND tenant_id=? and throws 400 E-1110 ("Unknown store or counter for this tenant") BEFORE any money logic — killing the old undefined-deref 500 and any cross-tenant reference. Both /bills and /bills/offline funnel through it. (E-1110, not the task-suggested E-1109, because E-1109 is already the payments-≠-payable code.) See the P2 section. |
| M6 ✅ | Pagination bypass — limit=-1 / huge / NaN returns whole tables → unbounded stall/OOM on /items,/bills,/audit. |
Clamp limit to [1,MAX], offset ≥ 0, reject NaN. |
| M7 ✅ | No request schema validation — as casts; wrong types 500 with full stack traces + absolute install paths. |
zod/valibot per route + a generic error handler that never leaks the stack; NODE_ENV=production. |
| M8 ✅ | Numeric edges slip through: fractional qty on integer units, qty > 2^53 (precision loss / ledger corruption), negative payment legs when the sum still matches. | Enforce unit-decimals; clamp qty/total < 2^53; require every payment leg > 0 with an allowlisted mode. |
| M9 ✅ REMEDIATED (2026-07-13) | doc-series fyShort 2-digit truncation → FYs 100 years apart collide, forcing UNIQUE violations that can block billing on a prefix. |
DONE: seriesPrefix now embeds the full 4-digit FY start year ("2026" vs "2126" no longer collide). To keep the GST 16-char rule at 6-digit sequences the counter↔FY / is dropped (the sequence - stays): ST1C22026-000123 (16 chars). doc_no format changes → dev reseed handles it; prod starts the new format at cutover. Caveat: store+counter codes must total ≤ 5 chars (the same tightness the old format had at C10). See the P2 section. |
| M10 ✅ REMEDIATED (2026-07-13) | Hardcoded demo owner credentials (thomas/PIN 9174/pw sims) auto-seed on an empty DB. |
DONE: seedIfEmpty/ensureBatchTrackingSeed run ONLY when SIMS_SEED_DEMO=1; a prod empty DB never auto-creates the known-credential owner (stays empty for onboarding). Dev keeps seeding via npm start → dev-start.mjs (sets the flag). Proven live: flag unset ⇒ 0 tenants, thomas login 401. See the P2 section. |
| M11 ✅ REMEDIATED (client PII) | Unencrypted customer PII in the offline IndexedDB (names, GSTINs, line items) on shared counter PCs. | POS client fixed: the queued bill now persists only customerId — no customerName/buyerGstin — and the server re-resolves the buyer's name/GSTIN/place-of-supply from that id at drain, exactly as for an online bill (repos.ts:296-298). Catalog + staff-roster cache is wiped on Lock/logout (clearCatalogCache), while un-drained queued bills are preserved. Verified in a real browser IndexedDB (queued record carries customerId only; injected PII dropped; cache cleared on lock; queue survived). Files: apps/pos/src/offline.ts, App.tsx onLock, BillingScreen.tsx queueOffline; test apps/pos/test/offline.test.ts. Residual (now closed): the queued lines/amounts sat in a plain IndexedDB until drained; the server tier they drain into is now SQLCipher-encrypted at rest (H3, closed 2026-07-13), so the standing at-rest exposure is removed end-to-end. |
| M12 ✅ (line cap) | Single-threaded synchronous server: one 5,200-line bill blocked /api/health 155 ms; no per-bill line cap. |
Cap lines.length; clamp result sizes (M6). |
| M13 ✅ REMEDIATED (2026-07-13) | Plaintext HTTP — bearer token rides the LAN cleartext; a sniffer unlocks H1 + all authed reads. The single highest-leverage fix. | DONE (available, not forced on dev): SIMS_TLS_CERT+SIMS_TLS_KEY ⇒ HTTPS (https.createServer); unset ⇒ HTTP dev. Dev-cert one-liner + "prod MUST supply certs" in server.ts. Prod deploy must set the certs. |
| M14 ✅ REMEDIATED (2026-07-13) | No helmet / security headers; X-Powered-By: Express banner. |
DONE: helmet on (SPA-compatible CSP; nosniff; Referrer-Policy: no-referrer) + explicit X-Frame-Options: DENY + frame-ancestors 'none'; x-powered-by already disabled (P0). Both apps load with zero CSP violations. |
| M15 ✅ NOTED (2026-07-13) | Service-worker cache-first over HTTP → MITM-injected bundle cached persistently (THEORETICAL; needs LAN MITM). | Closed once prod is HTTPS (M13). No dev-slice code change. |
LOW
- Bearer tokens + purchase-entry draft in web storage (readable by same-origin JS; not exploitable today since XSS is refuted). Move session to an
httpOnlycookie eventually.
Root cause & remediation shape
Almost every Critical/High traces to one architectural fact: the store-server was built as a trusted-client data layer (correct while the only client was our own verified React app in the browser) but it exposes a REST API that trusts the client for identity, business data, and input shape. The fix is one coherent theme, not 30 scattered patches:
Move the trust boundary into the server.
| Priority | Workstream | Closes |
|---|---|---|
| P0 ✅ DONE | Anchor every bill line to the item master (price, tax class, MRP, existence, unit) inside commitBill; a deviation requires a redeemed, bound override token |
C2, C3, H4, H5, H7, M8 |
| P0 ✅ DONE | Server-side authorization: per-route ActionCode middleware, deny-by-default |
C1, C4 |
| P0 ✅ DONE | Input validation layer (hand-rolled guards, no new deps) + generic error handler; clamp limit/offset; validate businessDate; cap lines | H6, M6, M7, M8, M12 |
| P1 ✅ DONE | Lock down the edges: /api/print allowlist, /bootstrap minimize, /auth/verify-pin behind auth + rate limit, session TTL/logout, tenant-scope store/counter (M5 closed with the P2 pass) |
H1, M1, M2, M4, M5 |
| P1 ✅ DONE | TLS on the store LAN (available via env; prod must supply certs) | M13, M15, amplifies all |
| P2 ✅ DONE (except H3) | Harden-before-cloud (D14): PIN pepper, audit hash chain, demo-seed gate, 4-digit-FY series, tenant-scope commit — plus offline-cache PII (M11) + helmet (M14) already closed | H2, M3, M5, M9, M10, M11, M14 |
| P2 ✅ DONE (2026-07-13) | Encryption at rest — full-DB SQLCipher via better-sqlite3-multiple-ciphers, keyed from a machine-scoped SIMS_DB_KEY; prod must supply the key (DPAPI/keychain) + tighten data-dir ACLs |
H3 |
Reviewer note on test artifacts
The PoCs left benign junk in the dev DB (test bills 000017–000023, two garbage-FY
bills, a corrupted Carry Bag stock balance, a self-clearing u4 lockout). Reset the dev DB
(delete apps/store-server/data/, restart) before any demo. No source was modified.