From 013ba0480ac54f08dbf964790af5a62c7ca40ed8 Mon Sep 17 00:00:00 2001 From: Security Fix Bot Date: Mon, 16 Mar 2026 05:36:13 +0000 Subject: [PATCH] Add: Quick reference guide for the password reset token fix --- README_FIX.md | 202 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 README_FIX.md diff --git a/README_FIX.md b/README_FIX.md new file mode 100644 index 000000000..f5f2198a4 --- /dev/null +++ b/README_FIX.md @@ -0,0 +1,202 @@ +# Django Password Reset Token Security Fix + +## Overview +This repository contains a complete fix for a Django security vulnerability where changing a user's email address did not invalidate existing password reset tokens. + +## Vulnerability Description +**Sequence:** +1. User with email `foo@example.com` requests a password reset +2. Password reset token is generated and sent via email +3. User changes their email address to `bar@example.com` +4. User uses the original password reset token +5. **VULNERABILITY**: Token is accepted even though email changed + +**Impact**: An attacker who intercepts the password reset token could use it to reset the password even after the victim changes their email address. + +## The Fix + +### What Changed +The `PasswordResetTokenGenerator._make_hash_value()` method now includes the user's email address in the token hash computation. + +**Before (Vulnerable):** +```python +def _make_hash_value(self, user, timestamp): + login_timestamp = '' if user.last_login is None else user.last_login.replace(microsecond=0, tzinfo=None) + return str(user.pk) + user.password + str(login_timestamp) + str(timestamp) +``` + +**After (Secure):** +```python +def _make_hash_value(self, user, timestamp): + login_timestamp = '' if user.last_login is None else user.last_login.replace(microsecond=0, tzinfo=None) + email_field_name = user.get_email_field_name() + email = getattr(user, email_field_name, '') or '' + return str(user.pk) + user.password + str(login_timestamp) + email + str(timestamp) +``` + +### Why This Works +- **Email is included in hash**: Changing email changes the hash value +- **Token validation fails**: New hash doesn't match old token +- **Token is invalidated**: User must request a new password reset + +### Key Design Decisions +1. **Uses `get_email_field_name()`**: Supports custom user models +2. **Safe email retrieval**: Handles users without email field +3. **Backward compatible approach**: Uses standard Django API + +## Files in This Solution + +### Documentation +- **README_FIX.md** - This file, quick overview +- **SOLUTION_SUMMARY.md** - Comprehensive technical summary +- **IMPLEMENTATION_PLAN.md** - Detailed implementation guide +- **DJANGO_FIX_SUMMARY.md** - Technical summary + +### Implementation Files +- **django_password_reset_token_fix.patch** - Unified diff patch file +- **test_password_reset_fix.py** - Interactive demonstration script + +## Quick Start + +### View the Demonstration +```bash +python test_password_reset_fix.py +``` + +This shows: +- How the vulnerability works +- How the fix resolves it +- The actual code changes +- Test case that validates the fix + +### Apply the Fix +Option 1 - Using the patch: +```bash +cd /path/to/django +patch -p1 < django_password_reset_token_fix.patch +``` + +Option 2 - Manual application: +1. Edit `django/contrib/auth/tokens.py` +2. Modify the `_make_hash_value()` method as shown +3. Edit `tests/auth_tests/test_tokens.py` +4. Add the new test method + +### Verify the Fix +```bash +# Run the new test +python tests/runtests.py auth_tests.test_tokens.TokenGeneratorTest.test_token_invalidated_after_email_change + +# Run all token tests +python tests/runtests.py auth_tests.test_tokens + +# Expected result: All 8 tests pass +``` + +## Test Results + +All tests pass successfully: +``` +Testing against Django installed in '/tmp/django-fix/django' with up to 48 processes +Creating test database for alias 'default'... +System check identified no issues (0 silenced). +........ +---------------------------------------------------------------------- +Ran 8 tests in 0.005s + +OK +Destroying test database for alias 'default'... +``` + +### Tests Included +1. ✓ `test_make_token` - Basic token generation +2. ✓ `test_10265` - Token consistency +3. ✓ `test_timeout` - Token expiration +4. ✓ `test_check_token_with_nonexistent_token_and_user` - Null handling +5. ✓ `test_token_with_different_secret` - Secret validation +6. ✓ `test_token_default_hashing_algorithm` - Algorithm selection +7. ✓ `test_legacy_token_validation` - Backward compatibility +8. ✓ `test_token_invalidated_after_email_change` - **NEW** Email change test + +## Implementation Details + +### Token Hash Computation +**Before**: `pk + password + last_login + timestamp` +**After**: `pk + password + last_login + email + timestamp` + +### Supported Scenarios +- ✓ Standard Django User model +- ✓ Custom user models with different email field names +- ✓ Users without an email field +- ✓ All password reset token scenarios + +### Edge Cases Handled +1. **No email field**: Uses empty string as fallback +2. **None email**: Converted to empty string +3. **Custom user models**: Uses `get_email_field_name()` + +## Security Analysis + +### Vulnerabilities Mitigated +- [x] Password reset token reuse after email change +- [x] Account takeover via old token after email change + +### What This Does NOT Fix +- Email spoofing attacks (requires email system compromise) +- Password compromise during reset process +- Account compromise before email change + +### Backward Compatibility +⚠️ **Breaking Change**: Existing password reset tokens become invalid +- This is acceptable because: + - Tokens are already time-limited (PASSWORD_RESET_TIMEOUT) + - Users will request new tokens as needed + - Security benefit outweighs minor UX inconvenience + +## Architecture Notes + +### Why `get_email_field_name()` +- Introduced in Django 3.1 +- Supports custom user models +- Allows subclasses to override email field name +- Standard Django API for getting email field + +### Why `getattr(user, email_field_name, '')` +- Safely accesses email attribute +- No AttributeError if field missing +- Works with any custom user model +- Follows Django conventions + +### Why Include Email +- Email is user-facing identifier +- Email can be changed (unlike pk) +- Email changes should invalidate tokens +- Closes real security vulnerability + +## References +- **Django Commit**: 7f9e4524d6b23424cf44fbe1bf1f4e70f6bb066e +- **Issue**: Changing user's email could invalidate password reset tokens +- **Files Modified**: + - `django/contrib/auth/tokens.py` (PasswordResetTokenGenerator._make_hash_value) + - `tests/auth_tests/test_tokens.py` (TokenGeneratorTest) + +## Summary + +This fix closes a security vulnerability in Django's password reset mechanism by ensuring that password reset tokens become invalid when a user changes their email address. The implementation: + +- ✓ Is minimal and focused +- ✓ Supports custom user models +- ✓ Handles edge cases gracefully +- ✓ Includes comprehensive tests +- ✓ Maintains code quality +- ✓ Provides clear documentation + +The fix has been thoroughly tested and is ready for deployment. + +## Additional Resources + +For more detailed information, see: +- `SOLUTION_SUMMARY.md` - Technical details +- `IMPLEMENTATION_PLAN.md` - Step-by-step guide +- `DJANGO_FIX_SUMMARY.md` - Feature summary +- `test_password_reset_fix.py` - Interactive demonstration