Modified lostpass.php and user.php to avoid a username leak when usin…#1606
Modified lostpass.php and user.php to avoid a username leak when usin…#1606mambax7 merged 9 commits intoXOOPS:masterfrom
Conversation
…g the reset password function.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaced three user-facing language constants with a single generic password-reset instruction and updated lostpass.php to use the non-personalized message constant (removed username interpolation from redirect and confirmation output). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce user-enumeration risk in the core password recovery flow by removing username disclosure from the post-reset redirect message and updating the corresponding language string.
Changes:
- Removed username interpolation when redirecting after a successful password reset in
lostpass.php. - Updated
_US_PWDMAILEDto a generic, non-identifying password reset message in the English user language file.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
htdocs/lostpass.php |
Stops passing the username into the redirect message after password reset. |
htdocs/language/english/user.php |
Replaces _US_PWDMAILED with a generic, non-enumerating message. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@htdocs/language/english/user.php`:
- Around line 63-69: Calls that wrap the language constants with sprintf/printf
and pass username args are still present; locate the usages where
sprintf(_US_PWDMAILED, $user->getVar('uname')), printf(_US_CONFMAIL,
$user->getVar('uname')) and printf(_US_CONFMAIL, $getuser[0]->getVar('uname'))
are used and remove the sprintf/printf wrapper and the username argument so the
code simply outputs the constant (e.g., use the constant directly with
echo/redirect/mail APIs), and delete any unnecessary $user->getVar('uname') /
$getuser[0]->getVar('uname') fetches to avoid extra work and prevent
reintroducing user enumeration.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@htdocs/lostpass.php`:
- Line 94: The echoed confirmation header in lostpass.php currently outputs raw
_US_CONFMAIL; update the echo in the block that prints the confirmation message
(the line using echo '<h4>' . _US_CONFMAIL . '</h4>'; ) to escape the value with
htmlspecialchars(_US_CONFMAIL, ENT_QUOTES, 'UTF-8') so the output is properly
HTML-escaped before rendering.
Updated user notifications in language files for clarity and consistency.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
|
Thank you, @kevinpetit |



This modifies the language string "_US_PWDMAILED" to no longer display the username of the user associated with the email and removes the username getting passed through in the header.
This fixes a security (OWASP user enumeration) issue where a hacker could use an e-mail to:
In modern applications, we should NEVER confirm if an account exists on a password reset page and certainly not give the user ID.
Further checks should still be added in another PR to add in rate limits and the trigger of a CAPTCHA upon multiple attempts.
Summary by CodeRabbit