Skip to content

fix: address 12 security findings from external report#75

Merged
viyoma merged 2 commits into
aws-samples:masterfrom
viyoma:fix/security-findings-remediation
Jun 18, 2026
Merged

fix: address 12 security findings from external report#75
viyoma merged 2 commits into
aws-samples:masterfrom
viyoma:fix/security-findings-remediation

Conversation

@viyoma

@viyoma viyoma commented May 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Addresses all 12 findings from the external security researcher report received via aws-security@ (tracked in V2223903651).

Dependency CVEs — resolved versions (lockfile)

# CVE Package Fix threshold Resolved Status
1 CVE-2024-39338 (SSRF) axios ≥1.7.4 1.16.1 Fixed
2 CVE-2025-27152 (absolute-URL handling) axios ≥1.8.2 1.16.1 Fixed
3 CVE-2025-58754 (DoS) axios ≥1.12.0 1.16.1 Fixed
4 CVE-2024-47764 (cookie injection) cookie ≥0.7.0 0.7.2 Fixed
5 CVE-2024-28849 (cross-origin credential leak) follow-redirects ≥1.15.6 1.16.0 Fixed

The package.json semver range (axios: "^1.7.9") allowed the lockfile to resolve to 1.16.1 at install time, which is what npm ci and the deployed Lambda see. PR #76 (form-data 4.0.6) landed as a Dependabot follow-up in the same window.

Code-level fixes (7 design + concurrency findings)

  • Incomplete JWT verification: added audience and issuer checks on the session JWT (was only verifying signature/algorithm)
  • Unsafe cookie attributes: TOKEN and NONCE cookies now set Secure, HttpOnly, SameSite
  • HTTPS enforcement: HTTP requests redirect to HTTPS
  • Predictable CSRF state: replaced request.uri state with per-flow random value
  • Shared PKCE per Lambda container: code_verifier / code_challenge generated per request via Object.assign() rather than module-level singleton
  • Open-redirect prevention + input validation: added on the redirect/callback path
  • Swallowed Okta error body: getNewJwtResponse now extracts error.response.data so operators see the actual upstream error

Customer Impact

  • HTTP requests redirect to HTTPS (transparent)
  • Existing sessions require one-time re-authentication (cookies now require Secure flag)
  • PKCE flow corrected (was broken before — same pair reused across all users on a warm container)

Testing

  • node -c auth.js — syntax valid
  • npm audit — 2 low-severity residual (elliptic via jwk-to-pem, no upstream fix available)
  • Added auth.test.js with verification tests for the security fixes

Supersedes

Dependabot PRs #62, #59, #58, #48, #29, #28

Design-level defects:
- Eliminate shared mutable state for per-request values (PKCE, nonce, state, code)
- Add cookie security attributes (httpOnly, secure, sameSite)
- Enforce HTTPS via CloudFront ViewerProtocolPolicy
- Prevent open redirect via state parameter validation
- Add input validation on authorization code parameter

Dependency CVEs:
- cookie ^0.3.1 -> ^0.7.2 (CVE-2024-47764)
- axios ^1.6.2 -> ^1.7.9 (CVE-2023-45857)
- jsonwebtoken ^9.0.0 -> ^9.0.2
- jwk-to-pem ^1.2.6 -> ^2.0.7
- Transitive semver ReDoS (CVE-2022-25883) fixed via audit

Also adds security disclaimer to README.
18 tests covering:
- Open redirect prevention
- PKCE per-request uniqueness
- Nonce validation
- Cookie security attributes
- Input validation on auth code
- Concurrency safety (no shared state mutation)
@viyoma viyoma merged commit 1a825ef into aws-samples:master Jun 18, 2026
@viyoma

viyoma commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Updated the PR description above to clarify the dependency CVE coverage.

The original description listed axios (CVE-2023-45857) against the package.json range ^1.7.9, which on its own does not satisfy CVE-2025-27152 (≥1.8.2) or CVE-2025-58754 (≥1.12.0). Those CVEs are addressed because the package-lock.json resolves to axios@1.16.1 — but that wasn't visible from the original description.

The description now lists the resolved lockfile versions and maps each of the 5 reporter-listed CVEs (axios x3, cookie, follow-redirects) to its fix threshold. Same table is on V2223903651 for the SOC CloudOps audit trail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants