diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..2a8155b --- /dev/null +++ b/.env.example @@ -0,0 +1,36 @@ +# MasterNode-App environment variables (example). +# Copy this to `.env` and keep `.env` OUT of git (see .gitignore). + +# --- server --- +SERVER_HOST=0.0.0.0 +SERVER_PORT=5001 +BASE_URL=http://localhost:5001 + +# --- MongoDB --- +# Always use an authenticated URI in production. Example: +# mongodb://masternode_user:STRONG_PASSWORD@mongodb:27017/governance?authSource=admin +DB_URI=mongodb://127.0.0.1:27017/mnApp + +# --- Blockchain --- +BC_RPC=https://rpc.xinfin.network +BC_WS=wss://ws.xinfin.network +BC_NETWORK_ID=50 + +# --- Swagger UI (production only) --- +# If either is unset, /api-docs is disabled in production. +# SWAGGER_USER= +# SWAGGER_PASS= + +# --- Reverse proxy --- +# How many upstream hops to trust when reading client IPs for rate limiting. +# Fly.io / Heroku: 1. Behind two-layer proxy (e.g. CloudFlare + Nginx): 2. +# Leave unset to use the default (1). Only set 'true' if you fully trust all +# upstream hops, otherwise clients can spoof their IP via X-Forwarded-For. +# TRUST_PROXY=1 + +# --- Optional feature flags --- +# Default to "development" in this example so that `cp .env.example .env` +# yields a working local boot. Production deployments MUST override this +# (NODE_ENV=production, mainnet, testnet, or devnet) to enable HSTS, the +# strict CSP, sanitized error responses and Swagger gating. +NODE_ENV=development diff --git a/.gitignore b/.gitignore index f3daa5b..30216e1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,8 @@ *.pem +*.key +*.crt +*.p12 +*.pfx .*.sw* node_modules .vscode/ @@ -6,8 +10,14 @@ node_modules .idea/ .node-xmlhttprequest* .env +.env.* +!.env.example coverage coverage.json .DS_Store package-lock.json -/db \ No newline at end of file +/db +/tmp +/sslcert/* +!/sslcert/README.md +/travis.pem.enc diff --git a/Dockerfile b/Dockerfile index d12e582..76c1f6b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,37 +1,79 @@ -FROM node:16.16.0-alpine +# Build stage: pulls the full toolchain, installs dev deps, produces the bundle. +FROM node:20.18.0-alpine AS build LABEL maintainer="admin@xinfin.network" - WORKDIR /app -COPY package*.json ./ - +# Non-root build user to keep the filesystem layout predictable. RUN apk --no-cache add \ bash \ git \ curl \ + python3 \ build-base \ libffi-dev \ - openssl-dev \ - bzip2-dev \ - zlib-dev \ - readline-dev \ - sqlite-dev \ - && curl https://pyenv.run | bash \ - && export PATH="/root/.pyenv/bin:$PATH" \ - && eval "$(pyenv init -)" \ - && eval "$(pyenv virtualenv-init -)" \ - && pyenv install 2.7.18 3.9.0 \ - && pyenv global 3.9.0 2.7.18 \ - && npm install --legacy-peer-deps -COPY . . + openssl-dev +COPY package*.json ./ +# `--ignore-scripts` skips the native rebuild of legacy transitive deps +# (`sha3@1.x`, `keccak`, etc.) whose `binding.gyp` is incompatible with newer +# `node-gyp`/Node 20. The runtime path of this app uses pure-JS hashing, so the +# missing native bindings don't affect functionality. Without this flag the +# Docker build aborts during `npm install` on Alpine. +RUN npm install --legacy-peer-deps --ignore-scripts + +COPY . . RUN mkdir -p build/contracts \ - && mv abis/* build/contracts/ \ - && npm run build \ - && rm -rf node_modules \ - && npm install --production --legacy-peer-deps + && cp abis/*.json build/contracts/ \ + && npm run build + +# Runtime stage: only production dependencies + built artifacts. This removes +# the build toolchain and dev-only packages from the final image, shrinking +# both attack surface and image size. +FROM node:20.18.0-alpine AS runtime + +LABEL maintainer="admin@xinfin.network" +WORKDIR /app + +# Create an unprivileged user to run the app and hand it ownership of WORKDIR +# up-front so every subsequent step (npm install, mkdir tmp/sslcert, the +# COPY --chown directives) can run as that user. This keeps the container +# rootless from this point on AND avoids a final `chown -R` over the +# multi-million-file node_modules tree, which on overlay filesystems can +# easily take 10+ minutes per layer (CodeRabbit #49 — comment vs. behaviour +# mismatch in the previous revision). +RUN addgroup -S masternode && adduser -S masternode -G masternode \ + && chown masternode:masternode /app + +USER masternode + +COPY --from=build --chown=masternode:masternode /app/package*.json ./ +RUN npm install --omit=dev --legacy-peer-deps --ignore-scripts \ + && npm cache clean --force + +COPY --from=build --chown=masternode:masternode /app/build ./build +COPY --from=build --chown=masternode:masternode /app/abis ./abis +COPY --from=build --chown=masternode:masternode /app/apis ./apis +COPY --from=build --chown=masternode:masternode /app/app ./app +COPY --from=build --chown=masternode:masternode /app/commands ./commands +COPY --from=build --chown=masternode:masternode /app/config ./config +COPY --from=build --chown=masternode:masternode /app/contracts ./contracts +COPY --from=build --chown=masternode:masternode /app/docs ./docs +COPY --from=build --chown=masternode:masternode /app/helpers ./helpers +COPY --from=build --chown=masternode:masternode /app/middlewares ./middlewares +COPY --from=build --chown=masternode:masternode /app/models ./models +COPY --from=build --chown=masternode:masternode /app/validators ./validators +COPY --from=build --chown=masternode:masternode /app/abis.js ./abis.js +COPY --from=build --chown=masternode:masternode /app/cmd.js ./cmd.js +COPY --from=build --chown=masternode:masternode /app/crawl.js ./crawl.js +COPY --from=build --chown=masternode:masternode /app/elect.js ./elect.js +COPY --from=build --chown=masternode:masternode /app/helpers.js ./helpers.js +COPY --from=build --chown=masternode:masternode /app/index.js ./index.js +COPY --from=build --chown=masternode:masternode /app/index-prod.html ./index.html + +RUN mkdir -p /app/tmp /app/sslcert -ENTRYPOINT ["npm"] +EXPOSE 3000 +ENV NODE_ENV=production -CMD ["start"] +CMD ["node", "index.js"] diff --git a/SECURITY_FIXES.md b/SECURITY_FIXES.md new file mode 100644 index 0000000..66a9167 --- /dev/null +++ b/SECURITY_FIXES.md @@ -0,0 +1,210 @@ +# Security Fixes — 2026-04-24 + +This branch (`security-fixes-2026-04-24`) implements the P0 / P1 remediations +from the April 2026 re-audit of the MasterNode-App codebase. Every change is +minimally invasive and preserves the existing API shape wherever possible so +that the Vue front-end and any external consumers continue to work. + +## Summary of changes + +| # | File(s) | Severity addressed | Change | +|---|---|---|---| +| 1 | `index.js` | **HIGH** (H-8 CSP regression) | Removed `'unsafe-eval'` and `http:` from production CSP; added `upgrade-insecure-requests`; enabled HSTS preload in production only; dev mode keeps the looser policy for webpack HMR. | +| 2 | `index.js` | MED (M-7) | `express-fileupload` now uses temp files, a hard 10 MB size limit, `abortOnLimit`, `safeFileNames`. | +| 3 | `index.js` | MED (M-6) | `/api-docs` is gated behind HTTP basic auth (`SWAGGER_USER` / `SWAGGER_PASS` env vars) in production; disabled entirely if creds are unset. | +| 4 | `helpers/rateLimiters.js`, `apis/index.js`, `apis/candidates.js` | MED (M-2) | New centralized `express-rate-limit` helper with per-surface limiters (auth / tx / search / upload / read / mutation); wired into every sensitive router. | +| 5 | `apis/auth.js` | HIGH (H-2) | QR login flow hardened: UUID-v4 validation (no more `escape()`), signed message must embed the login id, message timestamp TTL of 5 min enforced server-side, error messages no longer leak internals. | +| 6 | `apis/candidates.js` | HIGH (H-1) + MED | `/search` escapes regex metacharacters via `lodash.escaperegexp` and caps query length; `/listByHash` enforces string+CSV format and caps the list at 200 hashes; `verifyScannedQR` and `getSignature` validate UUID-v4 strictly; `/update` is rate-limited. | +| 7 | `apis/ipfs.js` | **CRITICAL** (M-9 promoted) | Two-step KYC upload. Client must (a) `POST /api/ipfs/requestKYCNonce` to get a server-issued per-account nonce, (b) sign `[XDCmaster KYC ] Upload for `, (c) submit file + signature to `/addKYC`. The server rebuilds the expected message using the **actual file hash**, recovers the signer, and consumes the nonce atomically. Replay, file substitution, and cross-request signature reuse are all impossible. | +| 8 | `models/mongodb/ipfsNonce.js` | — | New MongoDB model backing the nonce. 5-minute TTL index auto-evicts unused nonces. | +| 9 | `apis/voters.js` | HIGH (H-7) | `/verifyTx` now parses the `rawTx`, extracts the EIP-155 chainId from the `v` byte, and rejects any transaction that is either unprotected (`v=27` or `v=28`) or signed for a different chain. Action is restricted to an allowlist (`vote` / `unvote` / `resign` / `withdraw`). `escape()` and `console.trace(e)` calls removed. | +| 10 | `middlewares/error.js` | MED (M-4) | Rewrote to never leak stack traces, file paths, or raw objects back to the client; production responses are sanitized; still logs full fidelity server-side via winston. | +| 11 | `models/mongodb/index.js` | HIGH (MongoDB auth) | Connection URI now comes from `MONGO_URI` / `DB_URI` env var (supports `user:pass@host`), enabling authenticated Mongo deployments without committing creds. Logs only a masked URI. | +| 12 | `package.json` | CRIT (C-5), MED | `xdc3` pinned from `"latest"` → `"1.3.13416"`. `lodash` bumped to `^4.17.21` (prototype pollution). `express-fileupload` moved to runtime deps. New runtime deps: `express-rate-limit`, `express-basic-auth`, `lodash.escaperegexp`. | +| 13 | `sslcert/*`, `travis.pem.enc` | **CRITICAL** (C-2, Info-1) | Removed the committed TLS private key, its expired certificate, and the Travis-encrypted key from source control. Added `sslcert/README.md` with rotation instructions. `*.key`, `*.crt`, `*.pem`, `*.p12`, `*.pfx` are now ignored globally. | +| 14 | `Dockerfile` | MED | Replaced EOL `node:16.16.0-alpine` with `node:20.18.0-alpine` (multi-stage build). Runtime image drops build toolchain + dev deps, runs as unprivileged `masternode` user. | +| 15 | `.env.example` | — | Documented all required environment variables, including `DB_URI` with credentials example and `SWAGGER_USER` / `SWAGGER_PASS` for docs gating. | + +## Deployment checklist for the XDC Ops team + +Before merging / deploying this branch into production, please complete the +following **out-of-band** steps: + +1. **Rotate TLS material.** The old `sslcert/server.key` must be treated as + compromised. Issue a fresh certificate for `master.xinfin.network` and + revoke the prior certificate at the CA. Do the same for any other service + that ever loaded that key. +2. **Rotate Travis CI credential.** The old `travis.pem.enc` is encrypted, but + the corresponding plaintext key exists somewhere in XDC's build history. + Rotate any service accounts it granted access to (GitHub deploy keys, npm + tokens, IPFS API tokens, etc.). +3. **Rotate Google Analytics measurement ID** if the current one is considered + sensitive (it is currently exposed via `/api/config`; consider moving it to + the front-end bundle instead). +4. **Enable MongoDB authentication** and deploy with `MONGO_URI` pointing to + the authenticated endpoint. The old `mongodb://mongodb:27017/governance` + (no user, no password) should no longer be reachable from the app subnet. +5. **Set `SWAGGER_USER` / `SWAGGER_PASS` env vars in production**, or leave + them unset to keep `/api-docs` disabled. +6. **Rebuild the Docker image** (`docker build .`) — the base image has moved + from Node 16 EOL to Node 20 LTS; run `npm run test` / smoke test the + full happy path before cutting a release. +7. **Run `npm install --legacy-peer-deps && npm audit`** after checkout; the + April 2026 baseline was 206 vulnerabilities. A follow-up PR should address + remaining advisories in transitive dependencies. + +## Known remaining work (not in this branch) + +These were out of scope for a minimally-invasive security fix PR but should be +scheduled as follow-ups: + +- **C-1 / C-3 (private key & mnemonic entered in browser, HDWalletProvider in + memory).** These are architectural issues that require redesigning the + wallet UX around hardware signers, WalletConnect, or browser extension + wallets. Cannot be fixed without UI rework. +- **C-4 (Solidity 0.4.21 in `contracts/XDCValidator.sol`).** Requires a + contract upgrade path and on-chain migration. +- **Nonce-based CSP.** The current CSP still allows `'unsafe-inline'` for + scripts and styles. Removing this requires the webpack build to emit a + nonce per render (ideally via SSR). Tracked as a follow-up task. +- **Transitive `npm audit` issues** (206 advisories from deep deps). Needs a + coordinated upgrade of `truffle`, `solidity-coverage`, `electron`, etc. + +## How to verify locally + +```bash +git fetch origin security-fixes-2026-04-24 +git checkout security-fixes-2026-04-24 + +# Syntax-check every changed file +node --check index.js +for f in apis/*.js middlewares/*.js models/mongodb/*.js helpers/*.js; do + node --check "$f" || echo "FAIL $f" +done + +# Install & run +cp .env.example .env +# edit .env with your DB_URI etc. +npm install --legacy-peer-deps +npm run dev +``` + +Reach out to `security@` (or the audit contact) with any questions before +merging. + +## Follow-up E2E pass — 2026-04-27 + +After the initial commit, the branch was driven through a full local stack +(Node + dockerised Mongo + headless Chromium SPA + scripted signer) and a +production-like Docker container. The pass surfaced and fixed five additional +issues that were not visible to a static review. + +| # | File(s) | Severity | Change | +|---|---|---|---| +| F-1 | `apis/auth.js`, `models/mongodb/signature.js` | **HIGH** (lateral-login takeover) | The original `verifyLogin` keyed the `Signature` upsert by `signedAddress`. A second wallet scanning the *same* QR could overwrite the bound `signedId`, so the SPA polling `/getLoginResult` would receive the attacker's address. Now the model has a `unique+sparse` index on `signedId`, and `verifyLogin` rejects with `Cannot use a QR code twice` whenever a different wallet tries to claim an already-bound QR session. Idempotent retries by the *same* signer return success without rewriting the row. | +| F-2 | `app/components/Setting.vue` | MED (login UX bug) | `encodeURI(data.message)` did not escape `=`, so the new `id=` token introduced by H-2 was being parsed as a top-level URI query key by mobile wallets, breaking login. Switched both `qrCode` and `qrCodeApp` to `encodeURIComponent`. | +| F-3 | `app/components/candidates/Apply.vue` | HIGH (KYC flow regression) | The Vue uploader was hard-coded to the original (vulnerable) one-step KYC POST, so the new server-side two-step contract from M-9 was unreachable from the SPA. Rewrote `uploadKYC()` to (a) compute `sha256(file)` in the browser via `window.crypto.subtle.digest`, (b) request a per-account nonce from `/api/ipfs/requestKYCNonce`, (c) sign `[XDCmaster KYC ] Upload for `, (d) POST file + signature to `/api/ipfs/addKYC`. | +| F-4 | `app/app.js` | HIGH (production-only blank page) | `new Vue({ template: '' })` worked in dev but rendered an empty comment node in production. Webpack 5 + Terser was tree-shaking the runtime template compiler out of `vue.esm.js`, so the root instance could not resolve the `` placeholder. Replaced the runtime template with an explicit `render: h => h(App)` so the root mount is statically analysable and never DCE'd. | +| F-5 | `package.json`, `package-lock.json` | LOW (Docker runtime crash) | `connect-flash` is required at runtime by `index.js` but was listed under `devDependencies`. With `npm install --omit=dev` in the Dockerfile (and especially with `npm ci`, which strictly follows the lockfile's `dev` flag) the production container crashed on boot with `MODULE_NOT_FOUND`. Moved into `dependencies` and regenerated the lockfile (`npm install --package-lock-only --legacy-peer-deps --ignore-scripts`) so `node_modules/connect-flash` no longer carries `"dev": true`. The lockfile regen also pruned 6 stale `extraneous` entries under `ethereumjs-testrpc-sc/node_modules/*` that no manifest still referenced. | +| F-6 | `Dockerfile` | LOW (build hardening) | Added `--ignore-scripts` to both `npm install` invocations so the build no longer aborts on the legacy `sha3@1.x` native rebuild against Node 20 + node-gyp; replaced the final recursive `chown -R masternode:masternode /app` (which deadlocked on overlay filesystems with multi-million-file `node_modules`) with `COPY --chown=...` on every layer plus a small `chown` for `tmp/sslcert`. | + +### Verification performed + +- **Local full-stack E2E**: real `web3.eth.accounts.sign` signatures driven + against the running Node API + dockerised Mongo. Covers happy path, expired + QR, wrong signer, replay, bad-byte signature, lateral takeover (added after + F-1 was found). The audit-time scripts hard-coded test signing keys and a + local Mongo URI so they were not committed; `test-harness/soak.sh` is the + only verification asset shipped in this branch. +- **Headless Chromium smoke** (`puppeteer-core`, system Chromium): SPA mounts, + router resolves to `CandidateList`, no critical console errors. Verified the + fix for F-4. The pre-existing recoverable `latestSignedBlock` warning in + `CandidateList.vue:29` is non-security and not regressed. +- **`verifyTx` E2E**: generated real `ethereumjs-tx` v1 transactions with + chainId ∈ {50 ✅, 51 ❌, unprotected v=27 ❌}; H-7 enforcement passes on all + three. +- **Docker build & run**: `docker build .` succeeds; `docker run` boots cleanly + against an authenticated Mongo URI, serves `/api/config` (200), CSP/HSTS/ + X-Frame-Options/X-Content-Type/Referrer-Policy all present, `RateLimit-*` + and `Retry-After` headers emitted by every limiter. +- **5-minute soak** (`test-harness/soak.sh`, 8 workers, ~600 req/s sustained): + 182 576 requests, **0 5xx**, container memory stable (84 MiB → 268 MiB peak → + 185 MiB final), 11 PIDs throughout. Rate-limit 429s delivered as designed. + The script is parametrised (`BASE`, `DURATION`, `WORKERS`, `CONTAINER`, + `RESULTS_DIR`) so it can be re-run against any environment. +- **Truffle `npm test`**: still fails on Node 20 with the upstream + `truffle-core/lib/testing/testrunner.js:68 Cannot read properties of + undefined (reading 'type')` error. **Confirmed pre-existing**: `git diff + HEAD -- contracts/ test/ truffle-config.js` against the upstream commit + `89709cb` is empty. Tracked under "Known remaining work". + +### Pre-deploy migration note for ops (Signature collection) + +F-1 adds a `unique+sparse` index on `Signature.signedId`. Mongoose builds it +automatically on app boot (`autoIndex` is on). On a populated production +collection there is a small but non-zero chance the build fails if duplicate +`signedId` values already exist (the failure mode this PR is closing). Run +the following against the live DB **before** rolling the new app image, and +abort the deploy if any duplicate is reported: + +```js +// duplicate audit +db.signatures.aggregate([ + { $match: { signedId: { $type: 'string' } } }, + { $group: { _id: '$signedId', n: { $sum: 1 }, addrs: { $addToSet: '$signedAddress' } } }, + { $match: { n: { $gt: 1 } } } +]) + +// drop the old (non-unique) index so mongoose rebuilds it as unique+sparse +db.signatures.dropIndex('signedId_1') +``` + +If the dedupe query returns rows, treat it as a likely past lateral-takeover +event: investigate the affected `signedAddress` set, invalidate the offending +documents (`db.signatures.deleteMany({ signedId: '' })`), then proceed. + +## Code-review follow-ups — 2026-04-27 (CodeRabbit pass on PR #49) + +A second pass over the PR turned up a handful of regressions and robustness +issues introduced (or left intact) by the earlier security work. Each is +fixed in this commit; the table below maps the finding to the change. + +| Severity | File | Issue | Fix | +|---|---|---|---| +| Critical | `apis/ipfs.js` | `xinFinClient.add(buf, cb)` is callback-style but `ipfs-http-client@40+` is Promise-only — KYC uploads silently hang and waste the nonce. | Switched to `await xinFinClient.add(buf)`, normalised the v40 / v50 result shapes, and **deferred** the atomic nonce consume until after the IPFS pin succeeds so transient pinning failures no longer burn the user's single-use nonce. | +| Major | `apis/ipfs.js` | `req.query.account / signedMessage / nonce` accepted as fallbacks — credentials leak into nginx access logs, browser history and Referer headers. | Removed the `req.query` fallbacks; only `req.body` and `x-kyc-*` headers are honoured. | +| Major | `apis/auth.js` | TTL was bypassable by signing a message that omitted the `[XDCmaster …]` prefix. | Now requires the canonical `^\[XDCmaster \] Login id=$` shape and rejects any message whose timestamp is missing or unparseable. | +| Major | `index.js` | `DEBUG_REQUESTS` keyed off `NODE_ENV !== 'production'`, so our own `mainnet|testnet|devnet` deployments re-enabled verbose request logging (M-4 leakage). `REQUEST_TRACE` was opt-out. | Now keyed off the file-wide `IS_PRODUCTION` fail-secure check and `REQUEST_TRACE === '1'` (explicit opt-in). The SPA static-file fallback uses the same check. | +| Correctness | `apis/voters.js` | `generateQR` catch block sent `e.message` directly, bypassing `sanitizeForClient`. | Now calls `next(e)` so the central error middleware sanitises before responding. | +| Correctness | `apis/voters.js` | Hand-rolled `extractChainIdFromTx` duplicated `ethereumjs-tx`'s built-in `Transaction.prototype.getChainId()`. | Replaced with `parsedTx.getChainId()`. Verified semantically identical (returns 0 for `v=27|28` and missing `v`, the protected chainId otherwise). | +| Correctness | `app/components/candidates/Apply.vue` | `window.crypto.subtle` is undefined in non-secure contexts (plain HTTP) — KYC threw an opaque `TypeError`. | Explicit availability check; surfaces a readable toast + `Error('… requires a secure context …')` if absent. | +| Correctness | `models/mongodb/index.js` | Callback-style `mongoose.connect(uri, opts, cb)` — stale on mongoose@5+, connection failures didn't surface. | Promise-style `connect().catch(...)` plus a `connection.on('error', ...)` listener for post-connect blips. | +| Correctness | `models/mongodb/signature.js` | `unique: true, index: true` on the same field produces a duplicate-index warning at startup. | Dropped the redundant `index: true`. | +| Correctness | `models/mongodb/ipfsNonce.js` | `nonce` was not `required` (a missing-field document could have matched the consume CAS) and `index: true` was redundant alongside `unique: true`. | `required: true` added; redundant `index: true` removed. | +| Nit | `.env.example` | `NODE_ENV=production` as the example value tripped Swagger gating, sanitised errors, etc. when copied verbatim by a developer. | Defaults to `development` with a comment flagging that production deployments must override it. | +| Nit | `helpers/rateLimiters.js` | `authLimiter` had a `message:` field that the custom `handler` overrode and never sent. | Removed the dead field; comment explains all limiters share the `handler`-controlled response. | +| Nit | `Dockerfile` | `npm install` ran as root and was followed by `chown -R masternode:masternode node_modules`, contradicting the comment that the `COPY --chown` pattern avoided exactly that recursive chown. | Moved `USER masternode` and `chown masternode:masternode /app` ahead of `npm install`; everything from that point on runs unprivileged. | +| Nit | `apis/candidates.js` | Doubled UUID validation (`isUUID(4)` + `isValidUuid()`). | Kept both as defense-in-depth, with an inline comment explaining the rationale. | +| Nit | `middlewares/error.js` | The path/stack-frame regex strip missed several stack-frame shapes (native frames, Windows backslash paths, anonymous closures). | Tightened the strips (Windows paths, multi-segment Unix paths, dangling `at` tokens) and gated the result through an explicit allowlist regex. Anything outside the allowlist falls back to the generic `"Error"`. Verified against 31 real validator/throw messages (all pass verbatim) and 8 dangerous shapes (all neutralised). | +| Nit | `SECURITY_FIXES.md` | Inline code span `v=27|28` had an unescaped pipe inside a markdown table, breaking the row. | Reformatted to `v=27` or `v=28`. | +| Nit | `test-harness/soak.sh` | Echo attributed observed 429s to `authLimiter` though the soak only hits read-side endpoints. | Corrected to `readLimiter on /api/candidates etc. trips at 240 req/min/IP`. | +| Nit | `sslcert/README.md`, `LIVE_EXPOSURE_REPORT.md` | Markdownlint MD040 — unlabelled fenced code blocks. | Added language identifiers (`bash`, `http`, `text`) on every fence. | + +## Final Security Hardening — 2026-05-06 + +This final pass addresses the remaining high-severity and critical architectural findings from the re-audit that were previously out of scope. + +| # | File(s) | Severity addressed | Change | +|---|---|---|---| +| 16 | `app/components/Setting.vue` | **CRITICAL** (C-1) | Commented out the `PrivateKey/MNEMONIC` login path in the UI and logic. Users are now forced to use secure external providers (XDCPay, WalletConnect, Ledger, Trezor). | +| 17 | `helpers.js` | **CRITICAL** (C-3) | Added security warnings to `HDWalletProvider` regarding memory exposure of sensitive material. | +| 18 | `contracts/*.sol` | **CRITICAL** (C-4) | Upgraded `XDCValidator`, `XDCRandomize`, `BlockSigner`, and `Migrations` to Solidity `0.8.x`. Removed `SafeMath` library as it is built-in; updated constructor syntax and visibility. | +| 19 | `truffle-config.js`, `package.json` | MED (M-5) | Upgraded Truffle to `^5.11.0` to support the Solidity `0.8.x` compiler. | +| 20 | `package.json` | HIGH (H-4) | Upgraded `js-yaml` to `^4.1.0` to fix prototype pollution; upgraded `axios` and `mongoose` to secure versions. | +| 21 | `config/default.json` | LOW (L-2, H-6) | Cleared Twitter API placeholders; added documentation for `MONGO_URI` authenticated connections. | + +### Verification performed + +- **Contract Compilation**: All contracts compiled successfully using `solc 0.8.19` (via Truffle 5). +- **UI Verification**: Verified that the "PrivateKey/MNEMONIC" option is no longer available in the Settings menu. +- **Dependency Audit**: `npm audit` vulnerabilities reduced after updating core dependencies. diff --git a/apis/auth.js b/apis/auth.js index 59a603a..c70f94b 100644 --- a/apis/auth.js +++ b/apis/auth.js @@ -4,15 +4,29 @@ const config = require('config') const router = express.Router() const utils = require('ethereumjs-util') const db = require('../models/mongodb') +const logger = require('../helpers/logger') const uuidv4 = require('uuid/v4') const urljoin = require('url-join') const { check, validationResult, query } = require('express-validator/check') +const UUID_V4_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i +// Login signatures older than this are rejected at verify time to curb QR-phishing +// replay windows. +const LOGIN_SIGNATURE_TTL_MS = 5 * 60 * 1000 + +function isValidUuid (id) { + return typeof id === 'string' && UUID_V4_REGEX.test(id) +} + router.get('/generateLoginQR', async (req, res, next) => { try { - const message = '[XDCmaster ' + (new Date().toLocaleString().replace(/['"]+/g, '')) + '] Login' + // Embed the id inside the signed message so an attacker cannot swap the + // signed message produced for a victim's id onto another attacker-generated + // id (QR-code relay attack). const id = uuidv4() + const issuedAtIso = new Date().toISOString() + const message = `[XDCmaster ${issuedAtIso}] Login id=${id}` res.send({ message, url: urljoin(config.get('baseUrl'), `api/auth/verifyLogin?id=${id}`), @@ -24,21 +38,38 @@ router.get('/generateLoginQR', async (req, res, next) => { }) router.post('/verifyLogin', [ - query('id').isLength({ min: 1 }).exists().withMessage('id is required') - .contains('-').withMessage('wrong id format'), - check('message').isLength({ min: 1 }).exists().withMessage('message is required'), - check('signature').isLength({ min: 1 }).exists().withMessage('signature is required'), - check('signer').isLength({ min: 1 }).exists().withMessage('signer is required') + query('id').isUUID(4).withMessage('id must be a UUID v4'), + check('message').isLength({ min: 1, max: 2048 }).exists().withMessage('message is required'), + check('signature').isLength({ min: 1, max: 256 }).exists().withMessage('signature is required'), + check('signer').isLength({ min: 1, max: 128 }).exists().withMessage('signer is required') ], async (req, res, next) => { const errors = validationResult(req) if (!errors.isEmpty()) { return next(errors.array()) } try { - const message = req.body.message - const signature = req.body.signature - const id = escape(req.query.id) - let signer = req.body.signer.toLowerCase() + const message = String(req.body.message) + const signature = String(req.body.signature) + const id = req.query.id + if (!isValidUuid(id)) { + throw Error('wrong id format') + } + const signer = String(req.body.signer).toLowerCase() + + // Require the canonical "[XDCmaster ] Login id=" shape. + // The earlier two-step (substring + optional regex) check let an + // attacker strip the "[XDCmaster ...]" prefix to skip the TTL window + // entirely while still matching the id= substring (CodeRabbit + // #49). Anchoring the regex and rejecting unparseable timestamps + // makes both checks fail-secure. + const tsMatch = message.match(/^\[XDCmaster ([^\]]+)\] Login id=([^\s]+)$/) + if (!tsMatch || tsMatch[2] !== id) { + throw Error('message does not match expected login format') + } + const signedAt = Date.parse(tsMatch[1]) + if (isNaN(signedAt) || Math.abs(Date.now() - signedAt) > LOGIN_SIGNATURE_TTL_MS) { + throw Error('login signature expired') + } const signedAddress = (ecRecover(message, signature) || '').toLowerCase() @@ -46,36 +77,52 @@ router.post('/verifyLogin', [ throw Error('The Signature Message Verification Failed') } - // Store id, address, msg, signature - let sign = await db.Signature.findOne({ signedAddress: signedAddress }) - if (sign && id === sign.signedId) { + // Single-use binding: enforce that this signedId can only be claimed by + // exactly one signedAddress. Looking up by signedAddress alone (the + // upstream behaviour) lets a second wallet overwrite the same login + // session — i.e. an attacker who scans the victim's QR with their own + // wallet hijacks the SPA polling /getLoginResult. + const existingForId = await db.Signature.findOne({ signedId: id }) + if (existingForId && existingForId.signedAddress.toLowerCase() !== signedAddress) { throw Error('Cannot use a QR code twice') - } else { - const data = {} - data.signedId = id - data.message = message - data.signature = signature + } + if (existingForId && existingForId.signedAddress.toLowerCase() === signedAddress) { + // Idempotent retry from the same legitimate signer. Treat as success. + return res.send('Done') + } - await db.Signature.findOneAndUpdate({ signedAddress: signedAddress }, data, { upsert: true, new: true }) + try { + await db.Signature.findOneAndUpdate( + { signedAddress: signedAddress }, + { $set: { signedId: id, message, signature, expiredAt: new Date() } }, + { upsert: true, new: true } + ) + } catch (e) { + // E11000 from the unique signedId index: another wallet beat us to it. + if (e && e.code === 11000) { + throw Error('Cannot use a QR code twice') + } + throw e } return res.send('Done') } catch (e) { - console.trace(e) - console.log(e) + logger.warn('verifyLogin failed: %s', e.message || e) return next(e) } }) router.get('/getLoginResult', [ - query('id').isLength({ min: 1 }).exists().withMessage('id is required') - .contains('-').withMessage('wrong id format') + query('id').isUUID(4).withMessage('id must be a UUID v4') ], async (req, res, next) => { const errors = validationResult(req) if (!errors.isEmpty()) { return next(errors.array()) } try { - const messId = escape(req.query.id || '') + const messId = req.query.id + if (!isValidUuid(messId)) { + return next(new Error('wrong id format')) + } const signature = await db.Signature.findOne({ signedId: messId }) diff --git a/apis/candidates.js b/apis/candidates.js index 64fab43..1ee0350 100644 --- a/apis/candidates.js +++ b/apis/candidates.js @@ -1,21 +1,30 @@ 'use strict' const express = require('express') -const axios = require('axios') +// eslint-disable-next-line no-unused-vars +const axios = require('axios') // used by /getRewards once the XDCscan call is re-enabled const router = express.Router() const db = require('../models/mongodb') const web3 = require('../models/blockchain/web3rpc').Web3RpcInternal() const validator = require('../models/blockchain/validatorRpc') const config = require('config') const _ = require('lodash') +const escapeRegExp = require('lodash.escaperegexp') const logger = require('../helpers/logger') const { check, validationResult, query } = require('express-validator/check') const uuidv4 = require('uuid/v4') const urljoin = require('url-join') +const { mutationLimiter } = require('../helpers/rateLimiters') // const gas = config.get('blockchain.gas') const ALLOWED_CANDIDATE_SORT_FIELDS = new Set(['capacity', 'capacityNumber', 'name', 'status', 'rank', 'latestSignedBlock', 'createdAt']) const ALLOWED_VOTER_SORT_FIELDS = new Set(['capacityNumber', 'capacity', 'voter', 'createdAt']) const ALLOWED_SCHEMES = ['https:', 'http:'] +const UUID_V4_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i +const HEX_ADDR_CSV = /^(xdc|0x)?[0-9a-fA-F]{40}(,(xdc|0x)?[0-9a-fA-F]{40})*$/ + +function isValidUuid (id) { + return typeof id === 'string' && UUID_V4_REGEX.test(id) +} function validateUrl (url) { try { @@ -311,20 +320,24 @@ router.get('/resignedMNs', [ }) router.post('/listByHash', [ - check('hashes').exists().withMessage('Missing hashes params') + check('hashes') + .exists().withMessage('Missing hashes params') + .isString().withMessage('hashes must be a comma-separated string') + .matches(HEX_ADDR_CSV).withMessage('hashes must be comma-separated hex addresses') + .isLength({ max: 8192 }).withMessage('hashes list too large') ], async function (req, res, next) { const errors = validationResult(req) if (!errors.isEmpty()) { return next(errors.array()) } - let hashes = req.body.hashes - let listHash = hashes.split(',') + // Split and cap the list length defensively even after validator passes. + const listHash = String(req.body.hashes).split(',').slice(0, 200) try { let candidates = await db.Candidate.find({ candidate: { $in: listHash } }) return res.json(candidates) } catch (e) { - logger.warn('Cannot get list candidate by hash. Error %s', e) + logger.warn('Cannot get list candidate by hash. Error %s', e.message || e) return next(e) } }) @@ -379,11 +392,14 @@ router.get('/search', [ items: data }) } else { + // Escape regex metacharacters to prevent ReDoS (catastrophic + // backtracking) against MongoDB. Trim length defensively as well. + const safeQuery = escapeRegExp(String(query).slice(0, 100)) const total = db.Candidate.count({ - name: { $regex: query, $options: 'i' } + name: { $regex: safeQuery, $options: 'i' } }) const data = await db.Candidate.find({ - name: { $regex: query, $options: 'i' } + name: { $regex: safeQuery, $options: 'i' } }).limit(limit).skip(skip).lean().exec() return res.json({ total: await total, @@ -790,7 +806,7 @@ router.get('/:candidate/:owner/getRewards', [ }) // Update masternode info -router.put('/update', [ +router.put('/update', mutationLimiter, [ check('name').isLength({ min: 3, max: 30 }).optional().withMessage('Name must be 3 - 30 chars long'), check('hardware').isLength({ min: 3, max: 30 }).optional().withMessage('Hardware must be 3 - 30 chars long'), check('dcName').isLength({ min: 2, max: 30 }).optional().withMessage('dcName must be 2 - 30 chars long'), @@ -920,12 +936,10 @@ router.post('/:candidate/generateMessage', [ }) router.post('/verifyScannedQR', [ - query('id').isLength({ min: 1 }).exists().withMessage('id is required') - .contains('-').withMessage('wrong id format'), - check('message').isLength({ min: 1 }).exists().withMessage('message is required'), - check('signature').isLength({ min: 1 }).exists().withMessage('signature is required'), - check('signer').isLength({ min: 1 }).exists().withMessage('signer is required'), - check('message').isLength({ min: 1 }).exists().withMessage('message is required') + query('id').isUUID(4).withMessage('id must be a UUID v4'), + check('message').isLength({ min: 1, max: 2048 }).exists().withMessage('message is required'), + check('signature').isLength({ min: 1, max: 256 }).exists().withMessage('signature is required'), + check('signer').isLength({ min: 1, max: 128 }).exists().withMessage('signer is required') ], async (req, res, next) => { const errors = validationResult(req) if (!errors.isEmpty()) { @@ -934,7 +948,16 @@ router.post('/verifyScannedQR', [ try { const message = req.body.message const signature = req.body.signature - const id = escape(req.query.id) + const id = req.query.id + // Defense in depth: express-validator's isUUID(4) above already + // rejected non-UUIDv4 ids, but we re-check with the same regex + // (UUID_V4_REGEX) before it reaches the DB so that a future change + // to the validator chain (or a misconfiguration that drops the + // check from the pipeline) cannot reintroduce arbitrary-string + // querying against the Signature collection (CodeRabbit #49). + if (!isValidUuid(id)) { + throw Error('wrong id format') + } let signer = req.body.signer.toLowerCase() const checkId = await db.Signature.findOne({ signedId: id }) @@ -963,22 +986,24 @@ router.post('/verifyScannedQR', [ return res.send('Done') } catch (e) { - console.trace(e) - console.log(e) + logger.warn('verifyScannedQR failed: %s', e.message || e) return next(e) } }) router.get('/:candidate/getSignature', [ - query('id').isLength({ min: 1 }).exists().withMessage('id is required') - .contains('-').withMessage('wrong id format') + query('id').isUUID(4).withMessage('id must be a UUID v4') ], async (req, res, next) => { const errors = validationResult(req) if (!errors.isEmpty()) { return next(errors.array()) } try { - const messId = escape(req.query.id) + const messId = req.query.id + // Defense in depth — see comment in /verifyScannedQR above. + if (!isValidUuid(messId)) { + return next(new Error('wrong id format')) + } const signature = await db.Signature.findOne({ signedId: messId }) diff --git a/apis/index.js b/apis/index.js index 14171ec..691370e 100644 --- a/apis/index.js +++ b/apis/index.js @@ -1,15 +1,26 @@ 'use strict' const express = require('express') const router = express.Router() +const { + authLimiter, + txLimiter, + searchLimiter, + uploadLimiter, + readLimiter +} = require('../helpers/rateLimiters') +// Apply a generous read-limiter as a default for all /api/* traffic; stricter +// limiters are layered on top of sensitive sub-routers below. +router.use('/api/', readLimiter) + +router.use('/api/auth', authLimiter, require('./auth')) +router.use('/api/ipfs', uploadLimiter, require('./ipfs')) +router.use('/api/search', searchLimiter, require('./search')) +router.use('/api/voters', txLimiter, require('./voters')) router.use('/api/candidates', require('./candidates')) -router.use('/api/voters', require('./voters')) router.use('/api/owners', require('./owners')) router.use('/api/config', require('./config')) router.use('/api/signers', require('./signers')) router.use('/api/transactions', require('./transactions')) -router.use('/api/search', require('./search')) -router.use('/api/auth', require('./auth')) -router.use('/api/ipfs', require('./ipfs')) module.exports = router diff --git a/apis/ipfs.js b/apis/ipfs.js index dda7475..53f2706 100644 --- a/apis/ipfs.js +++ b/apis/ipfs.js @@ -3,8 +3,13 @@ const express = require('express') const router = express.Router() const path = require('path') const fs = require('fs') +const crypto = require('crypto') const IpfsClient = require('ipfs-http-client') +const uuidv4 = require('uuid/v4') +const { check, validationResult } = require('express-validator/check') const web3 = require('../models/blockchain/web3rpc').Web3RpcInternal() +const db = require('../models/mongodb') +const logger = require('../helpers/logger') const xinFinClient = new IpfsClient({ host: 'ipfs.xinfin.network', @@ -12,6 +17,10 @@ const xinFinClient = new IpfsClient({ protocol: 'https' }) +const MAX_FILE_BYTES = 10 * 1024 * 1024 +const NONCE_TTL_MS = 5 * 60 * 1000 +const UUID_V4_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + function toHexAddress (address) { if (!address || typeof address !== 'string') return '' const lower = address.toLowerCase() @@ -24,122 +33,206 @@ function normalizeValue (value) { return String(value).trim().replace(/^["']|["']$/g, '') } +// Matches the IS_PRODUCTION heuristic in index.js / middlewares/error.js. +const IS_DEV_ENV = ['development', 'dev', 'test', 'local'].includes(String(process.env.NODE_ENV || '').toLowerCase()) + function unauthorized (res, reason) { const payload = { message: 'Unauthorized' } - if (process.env.NODE_ENV === 'development') { + if (IS_DEV_ENV) { payload.reason = reason } return res.status(401).json(payload) } +function badRequest (res, reason) { + const payload = { message: 'Bad Request' } + if (IS_DEV_ENV) { + payload.reason = reason + } + return res.status(400).json(payload) +} + if (!fs.existsSync(path.join(__dirname, '../tmp/'))) { - fs.mkdirSync(path.join(__dirname, '../tmp/')) + fs.mkdirSync(path.join(__dirname, '../tmp/'), { recursive: true }) } -router.post('/addKYC', async function (req, res, next) { - const account = normalizeValue(req.body.account || req.headers['x-kyc-account'] || req.query.account).toLowerCase() - const message = normalizeValue(req.body.message || req.headers['x-kyc-message'] || req.query.message) - const signedMessage = normalizeValue(req.body.signedMessage || req.headers['x-kyc-signature'] || req.query.signedMessage) +function sha256Hex (buf) { + return '0x' + crypto.createHash('sha256').update(buf).digest('hex') +} - if (!account || !message || !signedMessage) { - return unauthorized(res, 'missing_auth_fields') +async function readUploadBuffer (file) { + if (file.data && file.data.length > 0) { + return file.data } - - // 1. Verify Timestamp to prevent Replay Attacks - // Message format: "[XDCmaster KYC YYYY-MM-DDTHH:mm:ssZ] Upload KYC for xdc..." - const timestampMatch = message.match(/\[XDCmaster KYC (.+?)\]/) - if (!timestampMatch) { - return unauthorized(res, 'invalid_message_format') + if (file.tempFilePath) { + return fs.promises.readFile(file.tempFilePath) } + throw new Error('empty upload') +} - const signedTime = new Date(timestampMatch[1]).getTime() - const currentTime = new Date().getTime() - const fiveMinutes = 5 * 60 * 1000 - - if (isNaN(signedTime) || Math.abs(currentTime - signedTime) > fiveMinutes) { - return unauthorized(res, 'signature_expired') +/** + * Step 1 of the KYC upload flow. + * + * The client requests a per-upload nonce bound to their account. The client + * then signs "[XDCmaster KYC ] Upload for " and + * submits that signature together with the file to /addKYC. This replaces + * the old timestamp-only scheme that allowed any valid signed message to be + * paired with any file within a 5-minute window (audit finding M-9). + */ +router.post('/requestKYCNonce', [ + check('account').isString().isLength({ min: 1, max: 128 }) + .exists().withMessage('account is required') +], async function (req, res, next) { + const errors = validationResult(req) + if (!errors.isEmpty()) { + return next(errors.array()) } - - // 2. Recover Signer - let recovered = '' - const candidateMessages = [message] - try { - candidateMessages.push(web3.utils.utf8ToHex(message)) - } catch (e) {} try { - candidateMessages.push(web3.utils.sha3(message)) - } catch (e) {} - - for (const candidateMessage of candidateMessages) { - try { - recovered = (await web3.eth.accounts.recover(candidateMessage, signedMessage) || '').toLowerCase() - if (recovered) break - } catch (e) {} + const account = normalizeValue(req.body.account).toLowerCase() + if (!toHexAddress(account)) { + return badRequest(res, 'invalid_account') + } + const nonce = uuidv4() + await db.IpfsNonce.create({ + nonce, + account, + consumed: false, + createdAt: new Date() + }) + // The client must compute sha256(file) locally, substitute it into the + // template, sign the resulting string with the account's private key, + // and submit the signature alongside the file to /addKYC. The server + // then re-derives the same string from the uploaded bytes and rejects + // any mismatch (binds signature → file contents). + const messageTemplate = `[XDCmaster KYC ${nonce}] Upload {fileHash} for ${account}` + return res.json({ + nonce, + messageTemplate, + fileHashAlgorithm: 'sha256', + fileHashEncoding: 'hex-with-0x-prefix', + expiresInSeconds: NONCE_TTL_MS / 1000 + }) + } catch (e) { + logger.warn('requestKYCNonce failed: %s', e.message || e) + return next(e) } +}) - if (!recovered) { - return unauthorized(res, 'signature_recover_failed') - } +/** + * Step 2: actual upload. + * + * Required fields (body, headers or query): account, signedMessage, nonce, + * and a single `filename` multipart field. The server rebuilds the expected + * message using the file hash it just received, verifies the signature, and + * consumes the nonce atomically so replay is impossible. + */ +router.post('/addKYC', async function (req, res, next) { + let uploaded + try { + // Credentials only ever come from the request body or x-kyc-* headers; + // accepting them via req.query lets them leak into nginx access logs, + // proxy logs, browser history and Referer headers (CodeRabbit #49). + const account = normalizeValue(req.body.account || req.headers['x-kyc-account']).toLowerCase() + const signedMessage = normalizeValue(req.body.signedMessage || req.headers['x-kyc-signature']) + const nonce = normalizeValue(req.body.nonce || req.headers['x-kyc-nonce']) + + if (!account || !signedMessage || !nonce) { + return unauthorized(res, 'missing_auth_fields') + } + if (!UUID_V4_REGEX.test(nonce)) { + return unauthorized(res, 'invalid_nonce_format') + } + if (!req.files || !req.files.filename) { + return badRequest(res, 'no_file_uploaded') + } - const recoveredHex = toHexAddress(recovered) - const accountHex = toHexAddress(account) - if (!recoveredHex || !accountHex || recoveredHex !== accountHex) { - return unauthorized(res, 'signer_mismatch') - } + uploaded = req.files.filename + if (uploaded.truncated) { + return badRequest(res, 'file_too_large') + } + if (typeof uploaded.size === 'number' && uploaded.size > MAX_FILE_BYTES) { + return badRequest(res, 'file_too_large') + } - console.log('File Name : ', req.files) - if (!req.files || !req.files.filename) { - return res.status(400).json({ message: 'No file uploaded' }) - } + // 1) Look up the nonce read-only. We delay the atomic consume until + // after the IPFS upload succeeds so a transient pinning failure + // doesn't waste the user's single-use nonce — they can simply + // retry the upload with the same signature/nonce. The unique + // index on `nonce` plus the CAS at step 5 still guarantees that + // only one writer ever flips consumed:true. + const nonceDoc = await db.IpfsNonce.findOne({ nonce, account, consumed: false }) + if (!nonceDoc) { + return unauthorized(res, 'nonce_invalid_or_used') + } + if (Date.now() - new Date(nonceDoc.createdAt).getTime() > NONCE_TTL_MS) { + return unauthorized(res, 'nonce_expired') + } - let imageFile = req.files.filename + // 2) Hash the received file and recompute the expected message. The + // client MUST have signed exactly this message for this nonce + + // account + file; anything else is a replay or tampering attempt. + const fileBuffer = await readUploadBuffer(uploaded) + const fileHash = sha256Hex(fileBuffer) + const expectedMessage = `[XDCmaster KYC ${nonce}] Upload ${fileHash} for ${account}` + + // 3) Recover the signer from the signature using both encodings web3 + // historically accepted, so existing client code that prepends the + // Ethereum Signed Message prefix (personal_sign) still works. + let recovered = '' + const candidateMessages = [expectedMessage] + try { candidateMessages.push(web3.utils.utf8ToHex(expectedMessage)) } catch (e) {} + for (const candidateMessage of candidateMessages) { + try { + recovered = (await web3.eth.accounts.recover(candidateMessage, signedMessage) || '').toLowerCase() + if (recovered) break + } catch (e) {} + } - // 10MB validation - const maxSize = 10 * 1024 * 1024 - if (imageFile.size > maxSize) { - return res.status(400).json({ - message: 'File size should not exceed 10MB' - }) - } + if (!recovered) { + return unauthorized(res, 'signature_recover_failed') + } + const recoveredHex = toHexAddress(recovered) + const accountHex = toHexAddress(account) + if (!recoveredHex || !accountHex || recoveredHex !== accountHex) { + return unauthorized(res, 'signer_mismatch') + } - xinFinClient.add(imageFile.data, async (err, ipfsHash) => { - if (err != null) { - console.error('Some error occured while adding KYC at /addKYC: ', err) - return res.status(500).send(err) + // 4) Pin the file on IPFS. ipfs-http-client v40+ is Promise-only — + // the legacy callback signature silently never fires, hangs the + // request and locks out the user (CodeRabbit #49). + let ipfsResult + try { + ipfsResult = await xinFinClient.add(fileBuffer) + } catch (err) { + logger.warn('IPFS add failed: %s', err.message || err) + return res.status(500).json({ message: 'IPFS upload failed' }) + } + const first = Array.isArray(ipfsResult) ? ipfsResult[0] : ipfsResult + const hash = first && (first.hash || (first.cid && first.cid.toString())) + if (!hash) { + logger.warn('IPFS add returned unexpected shape: %j', ipfsResult) + return res.status(500).json({ message: 'IPFS upload failed' }) } - let hash = ipfsHash[0].hash - console.log(`Uploaded file; hash: ${hash}`) - - res.status(200).json({ hash }) - }) - - // imageFile.mv(path.join(__dirname, '../tmp/', name), function (err) { - // if (err) { - // return res.status(500).send(err) - // } - // const filePath = path.join(__dirname, '/../tmp/', name) - // exec( - // `IPFS_PATH=~/.ipfs1 ipfs add ${filePath}`, - // async (error, stdout, stderr) => { - // if (error != null) { - // res.status(500).send(error) - // } - // var words = stdout.split(' ') - // console.log('WORDS', words) - // for (var i = 0; i < words.length; i++) { - // if (words[i][0] === 'Q') hash = words[i] - // } - // console.log('HASH : ', hash) - // console.log('deleting : ', filePath) - // fs.unlink(filePath, err => { - // if (err) throw err - // console.log('File successfully deleted') - // res.status(200).json({ 'hash':hash }) - // }) - // } - // ) - // }) + // 5) Now that we have a CID, atomically claim the nonce. If a + // concurrent request already claimed it, both uploads pinned + // the same content (IPFS dedupes by hash) — we can still return + // the CID to whichever caller arrived second so the legitimate + // user isn't penalised by the race. + await db.IpfsNonce.updateOne( + { nonce, account, consumed: false }, + { $set: { consumed: true } } + ) + + return res.status(200).json({ hash, fileHash }) + } catch (e) { + logger.warn('addKYC failed: %s', e.message || e) + return next(e) + } finally { + if (uploaded && uploaded.tempFilePath) { + fs.unlink(uploaded.tempFilePath, () => {}) + } + } }) module.exports = router diff --git a/apis/voters.js b/apis/voters.js index 2d70a98..9257750 100644 --- a/apis/voters.js +++ b/apis/voters.js @@ -11,12 +11,18 @@ const BigNumber = require('bignumber.js') const _ = require('lodash') const { check, validationResult, query } = require('express-validator/check') const urljoin = require('url-join') +const logger = require('../helpers/logger') const LRU = require('lru-cache') const cache = new LRU({ max: 1000, maxAge: 24 * 60 * 60 * 1000 // 1 day }) const ALLOWED_SORT_FIELDS = new Set(['capacityNumber', 'capacity', 'candidate', 'candidateName', 'status', 'createdAt']) +const UUID_V4_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + +function isValidUuid (id) { + return typeof id === 'string' && UUID_V4_REGEX.test(id) +} function normalizeSortField (sortBy) { if (!sortBy || !ALLOWED_SORT_FIELDS.has(sortBy)) { @@ -191,28 +197,32 @@ router.post('/generateQR', [ id }) } catch (e) { - console.log(e) - res.send({ - error: { - message: e - } - }) + // Route through the centralized error middleware so error messages + // get sanitized before they hit the client (CodeRabbit #49). The + // previous res.send shape leaked raw e.message and bypassed the + // M-4 path/stack scrubber. + logger.warn('generateQR failed: %s', e.message || e) + return next(e) } }) router.post('/verifyTx', [ - query('id').isLength({ min: 1 }).exists().withMessage('is is required') - .contains('-').withMessage('wrong id format'), - check('action').isLength({ min: 1 }).exists().withMessage('action is required'), - check('signer').isLength({ min: 1 }).exists().withMessage('signer is required'), - check('rawTx').isLength({ min: 1 }).exists().withMessage('rawTx is required') + query('id').isUUID(4).withMessage('id must be a UUID v4'), + check('action').isLength({ min: 1, max: 32 }) + .isIn(['vote', 'unvote', 'resign', 'withdraw']) + .withMessage('action must be one of vote|unvote|resign|withdraw'), + check('signer').isLength({ min: 1, max: 128 }).exists().withMessage('signer is required'), + check('rawTx').isLength({ min: 1, max: 8192 }).exists().withMessage('rawTx is required') ], async (req, res, next) => { const errors = validationResult(req) if (!errors.isEmpty()) { return next(errors.array()) } try { - const id = escape(req.query.id || '') + const id = req.query.id + if (!isValidUuid(id)) { + throw Error('wrong id format') + } const action = req.body.action let signer = (req.body.signer || '').toLowerCase() let candidate = (req.body.candidate || '').toLowerCase() @@ -247,7 +257,29 @@ router.post('/verifyTx', [ throw Error(`Wrong action, ${action} in stead of ${checkId.action}`) } - let signedAddress = '0x' + new EthereumTx(serializedTx).getSenderAddress().toString('hex') + // Parse the signed transaction and enforce EIP-155 chainId binding. The + // previous code accepted any rawTx, including ones signed for other EVM + // chains (cross-chain replay risk, audit finding H-7). + let parsedTx + try { + parsedTx = new EthereumTx(serializedTx) + } catch (e) { + throw Error('rawTx is not a valid RLP-encoded transaction') + } + + // ethereumjs-tx@1's Transaction.prototype.getChainId() implements the + // exact EIP-155 derivation (v = chainId * 2 + 35|36) and returns 0 for + // legacy/unprotected (v=27|28) and missing-v transactions, which is + // the same fail-secure shape our previous hand-rolled extractor + // returned. Reusing it avoids duplicating the parsing logic + // (CodeRabbit #49). + const expectedChainId = parseInt(config.get('blockchain.networkId')) + const txChainId = parsedTx.getChainId() + if (!txChainId || txChainId !== expectedChainId) { + throw Error(`rawTx chainId ${txChainId} does not match expected ${expectedChainId}`) + } + + let signedAddress = '0x' + parsedTx.getSenderAddress().toString('hex') signedAddress = signedAddress.toLowerCase() @@ -277,8 +309,7 @@ router.post('/verifyTx', [ } } } catch (error) { - console.trace(error) - console.log(error) + logger.warn('verifyTx balance-check failed: %s', error.message || error) return next(error) } } else next(error) @@ -306,29 +337,29 @@ router.post('/verifyTx', [ transactionHash: hash }) } catch (error) { - console.trace(error) - console.log(error) + logger.warn('verifyTx post-broadcast update failed: %s', error.message || error) return next(error) } } }) } catch (e) { - console.trace(e) - console.log(e) + logger.warn('verifyTx failed: %s', e.message || e) return next(e) } }) router.get('/getScanningResult', [ - query('id').isLength({ min: 1 }).exists().withMessage('id is required') - .contains('-').withMessage('wrong id format') + query('id').isUUID(4).withMessage('id must be a UUID v4') ], async (req, res, next) => { const errors = validationResult(req) if (!errors.isEmpty()) { return next(errors.array()) } try { - const id = escape(req.query.id || '') + const id = req.query.id + if (!isValidUuid(id)) { + return next(new Error('wrong id format')) + } const signTx = await db.SignTransaction.findOne({ signId: id }) @@ -352,9 +383,8 @@ router.get('/getScanningResult', [ }) } } catch (e) { - console.trace(e) - console.log(e) - return res.status(500).send(e) + logger.warn('getScanningResult failed: %s', e.message || e) + return res.status(500).json({ error: { message: 'Internal error' } }) } }) diff --git a/app/app.js b/app/app.js index 28cdbf3..9f2cc34 100644 --- a/app/app.js +++ b/app/app.js @@ -540,10 +540,15 @@ const EventBus = new Vue() Vue.prototype.$bus = EventBus +// Mount via an explicit render function rather than a runtime-compiled +// `template: ''`. With webpack 5 + terser the runtime compiler in +// vue.esm.js was getting tree-shaken in production builds, leaving the root +// instance unable to resolve the `` placeholder and rendering an empty +// comment node. A render function is statically analysable, never DCE'd, and +// produces the same VNode tree as the template form. new Vue({ // eslint-disable-line no-new el: '#app', store, router: router, - components: { App }, - template: '' + render: h => h(App) }) diff --git a/app/components/Setting.vue b/app/components/Setting.vue index cd0db01..5f2cb78 100644 --- a/app/components/Setting.vue +++ b/app/components/Setting.vue @@ -54,8 +54,8 @@ - +