Password recovery allows spurious additional emails
Description
The password reset/recovery functions allow spurious, additional email addresses to be used beyond the email address or personal address for the outbound recovery email.
It is unclear where exactly this is occurring, but the behavior is obviously insecure and undesirable.
Distribution Name and Version
Debian 10.13
FusionDirectory Version
FusionDirectory 1.3.1
PHP version used
PHP 7.3
Origin of php packages
Debian distribution packages
Steps to Reproduce
- Bad actor inputs spurious additional input beyond a known-good email address on the recovery.php page
- Password reset email is sent to the known-good email address as well as the bad actor
Expected behavior:
Input should be rejected or only the known-good matching email address should be used.
Actual behavior:
An email is sent to two locations - both directly to the user (as expected) and to the bad actor. This is send as a single email with two recipients.
Reproduces how often:
It is unclear how reproduceable this issue is. Inserting a basic comma does not pass the filtering requirement on the recovery form. However, we can confirm (from email logs and from the user themselves) that a recovery email was generated with two recipients.
Additional Information
The URL generated might be helpful for tracking this down: https://[Our FD installation]/recovery.php?uniq=[uniqueString]&login=[user]&email_address=[user]%40[our site]%00%2Ccirah11891%40botsoko.com
The "cirah11891%40botsoko.com" is the bad actor's email address (so I have no issues sharing it), although it's unclear how this a) passed validation checks or b) generated the 2nd recipient on the email.
After this incident, I have added some additional code to strip out any comma-separated email addresses and only use the first value (this is within the step2() function in class_passwordRecovery.inc), but even without that additional code, I was unable to replicate the issue directly.
My speculation is that something about the user-submitted string causes it to be ignored by the ldap filter, but the definition of $this->email_address = $_POST['email_address']; (line 314 in FD version 1.3.1) is never sanitized, allowing additional email address(es) to leak in to the process.