Skip to content

security: remove SHA-256 password fallback, CSPRNG for verification codes and keys - #148

Merged
NetworkTheoryAppliedResearchInstitute merged 2 commits into
mainfrom
security/harden-auth-primitives
Sep 14, 2026
Merged

NetworkTheoryAppliedResearchInstitute merged 2 commits into
mainfrom
security/harden-auth-primitives

Conversation

@NetworkTheoryAppliedResearchInstitute

Copy link
Copy Markdown
Collaborator

Fixes the credential-strength findings from CodeQL (alerts #10–#12):

  1. backend/utils/bcrypt.js — the module silently fell back to unsalted, single-round SHA-256 for password hashing whenever bcryptjs/bcrypt failed to load. bcryptjs is a declared dependency, so the fallback only ever fired on a broken install — which should fail loudly, not quietly store crackable hashes. It now throws with a clear message instead. No hash-format migration needed: any correctly-installed deployment was already using bcryptjs.
  2. backend/controllers/user_controller.js — 6-digit account verification codes were generated with Math.random(), which is predictable — a guessable code lets an attacker verify an account registered against someone else's email/phone. Now crypto.randomInt(100000, 1000000).
  3. backend/controllers/key_controller.js — generateMcElieseKey() produced ~13 chars of Math.random().toString(36); now 32 bytes of crypto.randomBytes (base64url). Note: despite the name, this is a random token generator, not an actual McEliece implementation — if real post-quantum keys are intended, that's a separate work item.

All three files pass node --check. Remaining open CodeQL findings (path injection in models/message.js, missing rate limiting on eight routes, workflow permissions) are deliberately not in this PR — happy to follow up separately.

🤖 Generated with Claude Code

…keys

- utils/bcrypt.js: the silent fallback hashed passwords with unsalted
  single-round SHA-256 when bcryptjs/bcrypt failed to load. bcryptjs is a
  declared dependency, so the fallback only ever fired on a broken install;
  now it throws instead of silently degrading. (CodeQL alerts #11, #12)
- user_controller.js: 6-digit verification codes came from Math.random(),
  which is predictable; now crypto.randomInt. (CodeQL alert #10 source)
- key_controller.js: generateMcElieseKey used Math.random().toString(36);
  now 32 bytes of crypto.randomBytes as base64url.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Jodson Graves <info@ntari.org>

@csecrestjr csecrestjr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update README.es.md:94:node server.js with this

starts the server (e.g., http://localhost:5000)

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