1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-08 19:26:48 +00:00

fix(logs): a typed username cannot forge a record in the file log

Failed logins are logged now, so S-16 applies: what a user types as a
username must not break a row or forge another record. The Logs page
shows each value in its own cell, escaped, and the writer cuts the
username to the 150-character field (Django's login form does not stop
a longer one). The file-log line written when a row cannot be saved
escaped only "\n"; it now escapes "\r" too, so a CR LF in a username
stays one line.
This commit is contained in:
catborise 2026-10-23 14:13:32 +03:00
parent b24746c1cb
commit 291690bb20
2 changed files with 32 additions and 1 deletions

View file

@ -24,7 +24,7 @@ def write(user, host, instance, message, success=None, ip=None):
except Exception as e:
try:
line = f"audit row not saved ({e!r}): {user} {host} {instance} success={success} ip={ip} {message}"
logger.warning(line.replace("\n", "\\n"))
logger.warning(line.replace("\r", "\\r").replace("\n", "\\n"))
except Exception:
pass

View file

@ -49,3 +49,34 @@ class LoginRowsTests(TestCase):
self.assertEqual(self.login("right-pass-1").status_code, 429)
self.assertEqual([r[2] for r in rows()][4:], ["Locked out", "Login failed"])
self.assertTrue(all(r[0] == "bob" and r[3] is False for r in rows()))
@override_settings(OTP_ENABLED=False)
class LoginRowInputTests(TestCase):
"""S-16: what a failed login writes cannot break the row or forge another record."""
def login(self, username):
return self.client.post(reverse("accounts:login"), {"username": username, "password": "wrong"})
def test_an_oversized_username_is_cut_to_the_field(self):
self.assertEqual(self.login("x" * 300).status_code, 200)
self.assertEqual(rows(), [("x" * 150, "-", "Login failed", False, "127.0.0.1")])
def test_a_crlf_username_stays_in_its_row(self):
forged = "bob\r\nadmin - Login"
self.login(forged)
self.assertEqual(rows(), [(forged, "-", "Login failed", False, "127.0.0.1")])
admin = get_user_model().objects.create_superuser("s16-admin", "s16@example.com", "pw")
self.client.force_login(admin)
page = self.client.get(reverse("admin:logs")).content.decode()
self.assertEqual(page.count("Login failed"), 1)
def test_a_crlf_username_is_one_line_in_the_file_log(self):
from unittest.mock import patch
with patch.object(Logs.objects, "create", side_effect=RuntimeError("db down")), \
self.assertLogs("logs.audit", "WARNING") as logged:
self.login("bob\r\nforged record")
[line] = logged.output
self.assertNotIn("\r", line)
self.assertNotIn("\n", line)