Skip to content

fix : add typeof string validation for password fields in user endpoints#2360

Open
tmdeveloper007 wants to merge 1 commit into
nisshchayarathi:mainfrom
tmdeveloper007:#2355
Open

fix : add typeof string validation for password fields in user endpoints#2360
tmdeveloper007 wants to merge 1 commit into
nisshchayarathi:mainfrom
tmdeveloper007:#2355

Conversation

@tmdeveloper007

@tmdeveloper007 tmdeveloper007 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Fixes #2355

Summary

The POST /api/users/change-password and DELETE /api/users/me endpoints extracted currentPassword and newPassword from the JSON body without validating that they are strings. Since both endpoints use .length for validation (e.g. newPassword.length < 8), arrays like ["p","a","s","s"] bypass those checks because arrays have a .length property.

Malformed payloads could reach bcrypt.compare() and bcrypt.hash() with non-string values, causing unhandled type errors or unpredictable coercion.

Changes

  • app/api/users/change-password/route.ts: Added typeof currentPassword !== string || typeof newPassword !== string guard before any other validation. Returns 400 if either field is not a string.
  • app/api/users/me/route.ts: Added typeof body.password !== string guard in the DELETE handler before calling bcrypt.compare().

Security Impact

Prevents type confusion attacks on sensitive authentication endpoints. All password fields are now strictly validated as string type before cryptographic operations.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced password validation for account changes and deletion operations to ensure passwords are valid strings before processing. Invalid password inputs now return appropriate error responses.

@vercel

vercel Bot commented Jun 16, 2026

Copy link
Copy Markdown

@tmdeveloper007 is attempting to deploy a commit to the Nisshchaya's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added the GSSoC'26 Part of GirlScript Summer of Code 2026 label Jun 16, 2026
@github-actions

Copy link
Copy Markdown

🎉 Thanks for your contribution, @tmdeveloper007!

Your PR has passed our automated GSSoC quality checks. Here's a quick summary:

Check Status
PR description ✅ Provided
PR title ✅ Meaningful
Linked issue ✅ Found
Change size ✅ Looks good (13 lines across 2 file(s))

A maintainer will review your PR soon. Please be patient and available for feedback. 💪

GSSoC'26 automation · Maintainer: @nisshchayarathi

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@tmdeveloper007, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 49 minutes and 59 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d2072163-bcf2-485e-857d-b68a884ba891

📥 Commits

Reviewing files that changed from the base of the PR and between 0e2db0a and b36f4a3.

📒 Files selected for processing (2)
  • app/api/users/change-password/route.ts
  • app/api/users/me/route.ts
📝 Walkthrough

Walkthrough

Two API route handlers receive explicit typeof string checks for password fields. The change-password POST route validates currentPassword and newPassword, and the account-deletion DELETE route validates body.password. Non-string values now return a 400 JSON error before any bcrypt operations are reached.

Changes

Password Input Type Validation

Layer / File(s) Summary
String type guards in change-password and account-delete routes
app/api/users/change-password/route.ts, app/api/users/me/route.ts
change-password POST now checks typeof currentPassword !== 'string' || typeof newPassword !== 'string' and returns 400 "Passwords must be valid strings"; account-delete DELETE checks typeof body.password !== 'string' and returns 400 "Password must be a valid string". Both guards run before any cryptographic calls.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested labels

bug, security, type:security, level:advanced

Poem

🐇 A bunny hopped through the code one day,
Found arrays where strings were meant to stay.
"Not on my watch!" the rabbit declared,
With typeof checks, the endpoints were repaired.
No object or array shall fool bcrypt's gate —
Only true strings may pass. The rest: 400 fate! 🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding typeof string validation for password fields in user endpoints.
Linked Issues check ✅ Passed The pull request fully addresses issue #2355 by implementing explicit typeof string validation for password fields in both endpoints as required.
Out of Scope Changes check ✅ Passed All changes are directly scoped to address the type confusion vulnerability in password fields across the two identified endpoints with no extraneous modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

🎉 Thanks for your contribution, @tmdeveloper007!

Your PR has passed our automated GSSoC quality checks. Here's a quick summary:

Check Status
PR description ✅ Provided
PR title ✅ Meaningful
Linked issue ✅ Found
Change size ✅ Looks good (13 lines across 2 file(s))

A maintainer will review your PR soon. Please be patient and available for feedback. 💪

GSSoC'26 automation · Maintainer: @nisshchayarathi

Prevents type confusion attacks where arrays or objects are passed as
password values. The type check is placed after presence/length guards
to preserve existing error messages for missing/invalid-length inputs.
Arrays bypass the .length guard ([x].length >= 1) and would reach
bcrypt as non-strings without this fix.

Fixes nisshchayarathi#2355
@github-actions

Copy link
Copy Markdown

🎉 Thanks for your contribution, @tmdeveloper007!

Your PR has passed our automated GSSoC quality checks. Here's a quick summary:

Check Status
PR description ✅ Provided
PR title ✅ Meaningful
Linked issue ✅ Found
Change size ✅ Looks good (19 lines across 2 file(s))

A maintainer will review your PR soon. Please be patient and available for feedback. 💪

GSSoC'26 automation · Maintainer: @nisshchayarathi

@tmdeveloper007

Copy link
Copy Markdown
Contributor Author

CI status: Build, Type Check, Lint, CodeQL, GSSoC checks all green. Note: Unit Tests job reports a pre-existing failure in app/api/auth/sessions/tests/route.test.ts due to a Jest/ESM module compatibility issue (jose/dist/browser/index.js). This failure exists on the upstream main branch and is unrelated to this PR. Ready for merge.

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

Labels

GSSoC'26 Part of GirlScript Summer of Code 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security] Type Confusion in password management endpoints leads to potential authentication bypass or DoS

1 participant