fix(auth): B-2.3 security review fixes — XSS escape + token replay
Targeted fixes for issues raised by code review on commit 37639a7
(B-2.3 DictIA email rebrand). All fixes verified against the Windows
manual driver: 16/16 tests pass (12 pre-existing + 4 new regression).
Critical:
- C1 Stored XSS in transactional emails: user.name (validated only on
Length(max=49), no character class) was rendered raw into the f-string
HTML body of verification + reset emails. Added html.escape on the
HTML branch; text body keeps the raw string (no XSS surface). Also
hardened the fallback chain to ((name or '').strip() or username or
'utilisateur').strip() so a None/whitespace name never produces
'Bonjour ,'.
- C2 Reflected XSS in templates/auth/check_email.html: the email value
from request.form was concatenated with literal '<strong>' tags then
fed through | safe, defeating Jinja's autoescape. Split the string so
template-author HTML stays literal and {{ email }} is autoescaped.
Used   for NBSP instead of '1 heure' | safe (more readable).
Important:
- I1 Dropped {{ message | safe }} on flash blocks in
forgot_password.html and reset_password.html (matches check_email.html).
No XSS today (flashes are static literals) but removes the landmine.
- I2 Password reset token replay: URLSafeTimedSerializer is stateless,
so the same valid link could be clicked twice within the 1h window.
Added a check that user.password_reset_token == token after the user
lookup — runs before BOTH GET (form render) and POST (password update).
The existing 'user.password_reset_token = None' on success now
actually invalidates the token.
- I5 MIMEText defaults to us-ascii, which Q-encodes accented French
characters and produces mojibake in some clients. Added explicit
'utf-8' charset on both text and html parts in _send_email.
New regression tests (tests/test_email_service_dictia.py):
- test_verification_email_falls_back_when_name_is_whitespace (I4)
- test_verification_email_handles_unicode_name (I5)
- test_verification_email_escapes_html_in_user_name (C1)
- test_check_email_template_escapes_email_in_response (C2)
Out of scope (per review): M1 (already addressed via solid-color
fallback), M2 (datetime.utcnow — pre-existing, separate cleanup),
M3 (Windows test driver — documented in tests file docstring),
M4-M6 (deferred).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -677,6 +677,15 @@ def reset_password(token):
|
|||||||
flash('Utilisateur introuvable.', 'danger')
|
flash('Utilisateur introuvable.', 'danger')
|
||||||
return redirect(url_for('auth.forgot_password'))
|
return redirect(url_for('auth.forgot_password'))
|
||||||
|
|
||||||
|
# Single-use token enforcement: URLSafeTimedSerializer is stateless, so
|
||||||
|
# without this check a valid token could be replayed within the 1h window.
|
||||||
|
# We invalidate the token on use by clearing user.password_reset_token, so
|
||||||
|
# a mismatch here means the token has already been consumed (or never set).
|
||||||
|
# Applies to BOTH GET (don't render the form) and POST (don't accept reset).
|
||||||
|
if not user.password_reset_token or user.password_reset_token != token:
|
||||||
|
flash('Le lien de réinitialisation est invalide ou expiré.', 'danger')
|
||||||
|
return redirect(url_for('auth.forgot_password'))
|
||||||
|
|
||||||
if request.method == 'POST':
|
if request.method == 'POST':
|
||||||
password = request.form.get('password')
|
password = request.form.get('password')
|
||||||
confirm_password = request.form.get('confirm_password')
|
confirm_password = request.form.get('confirm_password')
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import logging
|
|||||||
from email.mime.text import MIMEText
|
from email.mime.text import MIMEText
|
||||||
from email.mime.multipart import MIMEMultipart
|
from email.mime.multipart import MIMEMultipart
|
||||||
from datetime import datetime, timedelta
|
from datetime import datetime, timedelta
|
||||||
|
from html import escape as html_escape
|
||||||
from typing import Optional
|
from typing import Optional
|
||||||
|
|
||||||
from itsdangerous import URLSafeTimedSerializer, SignatureExpired, BadSignature
|
from itsdangerous import URLSafeTimedSerializer, SignatureExpired, BadSignature
|
||||||
@@ -133,13 +134,13 @@ def _send_email(to_email: str, subject: str, html_body: str, text_body: str = No
|
|||||||
msg['From'] = f"{config['from_name']} <{config['from_address']}>"
|
msg['From'] = f"{config['from_name']} <{config['from_address']}>"
|
||||||
msg['To'] = to_email
|
msg['To'] = to_email
|
||||||
|
|
||||||
# Add plain text version
|
# Add plain text version (explicit UTF-8 to prevent Q-encoding mojibake)
|
||||||
if text_body:
|
if text_body:
|
||||||
part1 = MIMEText(text_body, 'plain')
|
part1 = MIMEText(text_body, 'plain', 'utf-8')
|
||||||
msg.attach(part1)
|
msg.attach(part1)
|
||||||
|
|
||||||
# Add HTML version
|
# Add HTML version (explicit UTF-8 to prevent Q-encoding mojibake)
|
||||||
part2 = MIMEText(html_body, 'html')
|
part2 = MIMEText(html_body, 'html', 'utf-8')
|
||||||
msg.attach(part2)
|
msg.attach(part2)
|
||||||
|
|
||||||
# Connect to SMTP server
|
# Connect to SMTP server
|
||||||
@@ -309,15 +310,20 @@ def send_verification_email(user) -> bool:
|
|||||||
# Build verification URL
|
# Build verification URL
|
||||||
verify_url = url_for('auth.verify_email', token=token, _external=True)
|
verify_url = url_for('auth.verify_email', token=token, _external=True)
|
||||||
|
|
||||||
# Display name preferred over username; fallback when name is None/empty.
|
# Display name preferred over username; fallback chain handles None/empty
|
||||||
display_name = (getattr(user, 'name', None) or user.username).strip()
|
# name AND the schema-improbable case where username is also missing.
|
||||||
|
# HTML body MUST escape user-controlled name to prevent stored XSS;
|
||||||
|
# text body uses raw string (plaintext has no XSS surface).
|
||||||
|
raw_display_name = ((getattr(user, 'name', None) or '').strip() or user.username or 'utilisateur').strip()
|
||||||
|
display_name_html = html_escape(raw_display_name)
|
||||||
|
display_name_text = raw_display_name
|
||||||
|
|
||||||
subject = "Vérifiez votre courriel — DictIA"
|
subject = "Vérifiez votre courriel — DictIA"
|
||||||
|
|
||||||
content_html = f"""
|
content_html = f"""
|
||||||
<h2 style="color: #060d1a; margin: 0 0 24px 0; font-size: 24px; font-weight: 700;">Vérifiez votre adresse courriel</h2>
|
<h2 style="color: #060d1a; margin: 0 0 24px 0; font-size: 24px; font-weight: 700;">Vérifiez votre adresse courriel</h2>
|
||||||
|
|
||||||
<p style="color: #374151; margin: 0 0 16px 0; font-size: 16px;">Bonjour {display_name},</p>
|
<p style="color: #374151; margin: 0 0 16px 0; font-size: 16px;">Bonjour {display_name_html},</p>
|
||||||
|
|
||||||
<p style="color: #374151; margin: 0 0 24px 0; font-size: 16px;">
|
<p style="color: #374151; margin: 0 0 24px 0; font-size: 16px;">
|
||||||
Bienvenue chez DictIA. Pour terminer votre inscription et commencer à transcrire, vérifiez votre adresse courriel.
|
Bienvenue chez DictIA. Pour terminer votre inscription et commencer à transcrire, vérifiez votre adresse courriel.
|
||||||
@@ -338,7 +344,7 @@ def send_verification_email(user) -> bool:
|
|||||||
</div>
|
</div>
|
||||||
"""
|
"""
|
||||||
|
|
||||||
content_text = f"""Bonjour {display_name},
|
content_text = f"""Bonjour {display_name_text},
|
||||||
|
|
||||||
Bienvenue chez DictIA. Pour terminer votre inscription et commencer à transcrire, vérifiez votre adresse courriel.
|
Bienvenue chez DictIA. Pour terminer votre inscription et commencer à transcrire, vérifiez votre adresse courriel.
|
||||||
|
|
||||||
@@ -376,15 +382,20 @@ def send_password_reset_email(user) -> bool:
|
|||||||
# Build reset URL
|
# Build reset URL
|
||||||
reset_url = url_for('auth.reset_password', token=token, _external=True)
|
reset_url = url_for('auth.reset_password', token=token, _external=True)
|
||||||
|
|
||||||
# Display name preferred over username; fallback when name is None/empty.
|
# Display name preferred over username; fallback chain handles None/empty
|
||||||
display_name = (getattr(user, 'name', None) or user.username).strip()
|
# name AND the schema-improbable case where username is also missing.
|
||||||
|
# HTML body MUST escape user-controlled name to prevent stored XSS;
|
||||||
|
# text body uses raw string (plaintext has no XSS surface).
|
||||||
|
raw_display_name = ((getattr(user, 'name', None) or '').strip() or user.username or 'utilisateur').strip()
|
||||||
|
display_name_html = html_escape(raw_display_name)
|
||||||
|
display_name_text = raw_display_name
|
||||||
|
|
||||||
subject = "Réinitialiser votre mot de passe — DictIA"
|
subject = "Réinitialiser votre mot de passe — DictIA"
|
||||||
|
|
||||||
content_html = f"""
|
content_html = f"""
|
||||||
<h2 style="color: #060d1a; margin: 0 0 24px 0; font-size: 24px; font-weight: 700;">Réinitialiser votre mot de passe</h2>
|
<h2 style="color: #060d1a; margin: 0 0 24px 0; font-size: 24px; font-weight: 700;">Réinitialiser votre mot de passe</h2>
|
||||||
|
|
||||||
<p style="color: #374151; margin: 0 0 16px 0; font-size: 16px;">Bonjour {display_name},</p>
|
<p style="color: #374151; margin: 0 0 16px 0; font-size: 16px;">Bonjour {display_name_html},</p>
|
||||||
|
|
||||||
<p style="color: #374151; margin: 0 0 24px 0; font-size: 16px;">
|
<p style="color: #374151; margin: 0 0 24px 0; font-size: 16px;">
|
||||||
Nous avons reçu une demande de réinitialisation pour votre compte DictIA. Cliquez sur le bouton ci-dessous pour créer un nouveau mot de passe.
|
Nous avons reçu une demande de réinitialisation pour votre compte DictIA. Cliquez sur le bouton ci-dessous pour créer un nouveau mot de passe.
|
||||||
@@ -405,7 +416,7 @@ def send_password_reset_email(user) -> bool:
|
|||||||
</div>
|
</div>
|
||||||
"""
|
"""
|
||||||
|
|
||||||
content_text = f"""Bonjour {display_name},
|
content_text = f"""Bonjour {display_name_text},
|
||||||
|
|
||||||
Nous avons reçu une demande de réinitialisation pour votre compte DictIA. Cliquez sur le lien ci-dessous pour créer un nouveau mot de passe :
|
Nous avons reçu une demande de réinitialisation pour votre compte DictIA. Cliquez sur le lien ci-dessous pour créer un nouveau mot de passe :
|
||||||
|
|
||||||
|
|||||||
@@ -16,11 +16,11 @@
|
|||||||
|
|
||||||
<p class="text-sm text-brand-navy/70 mb-6">
|
<p class="text-sm text-brand-navy/70 mb-6">
|
||||||
{% if action == 'password_reset' %}
|
{% if action == 'password_reset' %}
|
||||||
{{ "Si un compte DictIA existe pour <strong>" ~ email ~ "</strong>, vous recevrez un courriel avec un lien pour réinitialiser votre mot de passe. Le lien expire dans 1 heure." | safe }}
|
Si un compte DictIA existe pour <strong>{{ email }}</strong>, vous recevrez un courriel avec un lien pour réinitialiser votre mot de passe. Le lien expire dans 1 heure.
|
||||||
{% elif action == 'verification_required' %}
|
{% elif action == 'verification_required' %}
|
||||||
{{ "Vérifiez votre boîte de réception à <strong>" ~ email ~ "</strong>. Si vous ne recevez rien, demandez un nouveau courriel ci-dessous." | safe }}
|
Vérifiez votre boîte de réception à <strong>{{ email }}</strong>. Si vous ne recevez rien, demandez un nouveau courriel ci-dessous.
|
||||||
{% else %}
|
{% else %}
|
||||||
{{ "Nous avons envoyé un lien de vérification à <strong>" ~ email ~ "</strong>. Cliquez dessus pour activer votre compte. Le lien expire dans 24 heures." | safe }}
|
Nous avons envoyé un lien de vérification à <strong>{{ email }}</strong>. Cliquez dessus pour activer votre compte. Le lien expire dans 24 heures.
|
||||||
{% endif %}
|
{% endif %}
|
||||||
</p>
|
</p>
|
||||||
|
|
||||||
|
|||||||
@@ -17,7 +17,7 @@
|
|||||||
{% elif category == 'warning' %}bg-amber-50 text-amber-900 border border-amber-200
|
{% elif category == 'warning' %}bg-amber-50 text-amber-900 border border-amber-200
|
||||||
{% elif category == 'success' %}bg-green-50 text-green-900 border border-green-200
|
{% elif category == 'success' %}bg-green-50 text-green-900 border border-green-200
|
||||||
{% else %}bg-blue-50 text-blue-900 border border-blue-200{% endif %}">
|
{% else %}bg-blue-50 text-blue-900 border border-blue-200{% endif %}">
|
||||||
{{ message | safe }}
|
{{ message }}
|
||||||
</div>
|
</div>
|
||||||
{% endfor %}
|
{% endfor %}
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
|||||||
@@ -17,7 +17,7 @@
|
|||||||
{% elif category == 'warning' %}bg-amber-50 text-amber-900 border border-amber-200
|
{% elif category == 'warning' %}bg-amber-50 text-amber-900 border border-amber-200
|
||||||
{% elif category == 'success' %}bg-green-50 text-green-900 border border-green-200
|
{% elif category == 'success' %}bg-green-50 text-green-900 border border-green-200
|
||||||
{% else %}bg-blue-50 text-blue-900 border border-blue-200{% endif %}">
|
{% else %}bg-blue-50 text-blue-900 border border-blue-200{% endif %}">
|
||||||
{{ message | safe }}
|
{{ message }}
|
||||||
</div>
|
</div>
|
||||||
{% endfor %}
|
{% endfor %}
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
|||||||
@@ -266,6 +266,112 @@ def test_check_email_template_extends_marketing_base():
|
|||||||
_clear_smtp_env()
|
_clear_smtp_env()
|
||||||
|
|
||||||
|
|
||||||
|
def test_verification_email_falls_back_when_name_is_whitespace():
|
||||||
|
"""Empty/whitespace name must NOT produce 'Bonjour ,' — falls back to username."""
|
||||||
|
with app.test_request_context('/'):
|
||||||
|
_set_smtp_env()
|
||||||
|
db.create_all()
|
||||||
|
try:
|
||||||
|
user = User(username='claire42', email='claire@example.qc.ca',
|
||||||
|
password='x' * 60, name=' ', email_verified=False)
|
||||||
|
db.session.add(user)
|
||||||
|
db.session.commit()
|
||||||
|
with patch('src.services.email._send_email', return_value=True) as mock_send:
|
||||||
|
from src.services.email import send_verification_email
|
||||||
|
send_verification_email(user)
|
||||||
|
args, _ = mock_send.call_args
|
||||||
|
_, _, html_body, text_body = args
|
||||||
|
assert 'Bonjour ,' not in html_body
|
||||||
|
assert 'Bonjour claire42' in html_body
|
||||||
|
assert 'Bonjour claire42' in text_body
|
||||||
|
finally:
|
||||||
|
db.session.rollback()
|
||||||
|
db.drop_all()
|
||||||
|
_clear_smtp_env()
|
||||||
|
|
||||||
|
|
||||||
|
def test_verification_email_handles_unicode_name():
|
||||||
|
"""Accented French names must round-trip through email without mojibake."""
|
||||||
|
with app.test_request_context('/'):
|
||||||
|
_set_smtp_env()
|
||||||
|
db.create_all()
|
||||||
|
try:
|
||||||
|
user = User(username='francois', email='francois@example.qc.ca',
|
||||||
|
password='x' * 60, name='François Mélanie',
|
||||||
|
email_verified=False)
|
||||||
|
db.session.add(user)
|
||||||
|
db.session.commit()
|
||||||
|
with patch('src.services.email._send_email', return_value=True) as mock_send:
|
||||||
|
from src.services.email import send_verification_email
|
||||||
|
send_verification_email(user)
|
||||||
|
args, _ = mock_send.call_args
|
||||||
|
_, _, html_body, text_body = args
|
||||||
|
assert 'Bonjour François Mélanie' in html_body
|
||||||
|
assert 'Bonjour François Mélanie' in text_body
|
||||||
|
finally:
|
||||||
|
db.session.rollback()
|
||||||
|
db.drop_all()
|
||||||
|
_clear_smtp_env()
|
||||||
|
|
||||||
|
|
||||||
|
def test_verification_email_escapes_html_in_user_name():
|
||||||
|
"""user.name with HTML payload must be escaped in HTML body, raw in text body.
|
||||||
|
|
||||||
|
Regression test for C1 (stored XSS). A signup with name='<img onerror=...>'
|
||||||
|
persists the payload — without escape it executes when the verification
|
||||||
|
email renders.
|
||||||
|
"""
|
||||||
|
with app.test_request_context('/'):
|
||||||
|
_set_smtp_env()
|
||||||
|
db.create_all()
|
||||||
|
try:
|
||||||
|
payload = '<img src=x onerror=alert(1)>'
|
||||||
|
user = User(username='attacker', email='attacker@x.ca',
|
||||||
|
password='x' * 60, name=payload, email_verified=False)
|
||||||
|
db.session.add(user)
|
||||||
|
db.session.commit()
|
||||||
|
with patch('src.services.email._send_email', return_value=True) as mock_send:
|
||||||
|
from src.services.email import send_verification_email
|
||||||
|
send_verification_email(user)
|
||||||
|
args, _ = mock_send.call_args
|
||||||
|
_, _, html_body, text_body = args
|
||||||
|
# HTML body MUST escape the payload
|
||||||
|
assert payload not in html_body, \
|
||||||
|
'Raw HTML payload leaked into HTML email body!'
|
||||||
|
assert '<img src=x onerror=alert(1)>' in html_body
|
||||||
|
# Text body keeps the raw string (it's plaintext, no XSS surface)
|
||||||
|
assert payload in text_body
|
||||||
|
finally:
|
||||||
|
db.session.rollback()
|
||||||
|
db.drop_all()
|
||||||
|
_clear_smtp_env()
|
||||||
|
|
||||||
|
|
||||||
|
def test_check_email_template_escapes_email_in_response():
|
||||||
|
"""email value rendered into check_email.html must be HTML-escaped.
|
||||||
|
|
||||||
|
Regression test for C2 (reflected XSS). Posting a script payload to
|
||||||
|
/forgot-password reflected it unescaped via concat-then-safe pattern.
|
||||||
|
"""
|
||||||
|
with app.app_context():
|
||||||
|
app.config['WTF_CSRF_ENABLED'] = False
|
||||||
|
_set_smtp_env()
|
||||||
|
db.create_all()
|
||||||
|
try:
|
||||||
|
client = app.test_client()
|
||||||
|
payload = '<script>alert(1)</script>'
|
||||||
|
resp = client.post('/forgot-password', data={'email': payload})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
body = resp.data.decode('utf-8')
|
||||||
|
assert payload not in body, \
|
||||||
|
'Raw <script> payload leaked into rendered HTML!'
|
||||||
|
assert '<script>alert(1)</script>' in body
|
||||||
|
finally:
|
||||||
|
db.session.rollback()
|
||||||
|
db.drop_all()
|
||||||
|
_clear_smtp_env()
|
||||||
|
|
||||||
|
|
||||||
def test_resend_verification_rate_limited_per_user():
|
def test_resend_verification_rate_limited_per_user():
|
||||||
"""can_resend_verification returns (False, remaining) within the 60s cooldown."""
|
"""can_resend_verification returns (False, remaining) within the 60s cooldown."""
|
||||||
with app.app_context():
|
with app.app_context():
|
||||||
|
|||||||
Reference in New Issue
Block a user