Closed missytake closed 1 year ago
Overall I think validation of user input is not the right place to fix it. It's always dangerous to add this kind of validation because you have to copy-paste it to every place where the user inputs something, taking into account all the uses of user input which happen in a completely different place.
I think get_user()
function should be fixed instead to URL-escape the address when it is used in the GET request.
I moved the email sanitization check into the mailcow.py method - it doesn't change anything in the current state actually, because the only place where it's executed apart from tests is in the add_email_account
method where the check happened before, but now it's a bit invisible to the add_email_account
function that the sanitization actually happens. So if we refactor add_email_account
again, it might get lost and maybe we have unsanitized email addresses somewhere in mailadm again. So it's better in protecting against MER-01-005 now, but worse in protecting against MER-01-004^^ I'm fine with both alternatives.
Maybe I should have split this up into two different PRs - one is about validating emails (which get passed to the mailcow API: MER-01-004 and MER-01-005), the other is about validating token names (which gets passed to the tokenQRcode.png file name: MER-01-001).
For newcomers & archiving purposes - there is a vaguely related thread about bot commands security.
Not critical, as this input is coming from trusted users - nevertheless it should be fixed.