Input sanitisation

[chrulle]

This morning our Indico instance (http://indico.nbi.ku.dk) was hit by an automated scan (Nessus) by DKCERT (https://www.cert.dk/)

This flooded our admin inboxes with error mails, because user input is not properly checked and escaped. "Luckily" it was caught by other libraries(sqlalchemy, datetime), but only as raw ValueError exceptions.

User input should be properly sanitised and escaped before being passed to the DB. Alternatively exceptions like this should be caught and trigger a user error (and maybe a warning in the logs). It should not trigger a python "crash".

Below I've included a couple of interesting stack traces showing the issue.

2017-11-06` 07:45:38,028  d4a07653228b45d5  indico.flask - ERROR errors.py:101 -- A string literal cannot contain NUL (0x00) characters.

Traceback (most recent call last):
  File "/opt/indico/.venv/lib/python2.7/site-packages/flask/app.py", line 1612, in full_dispatch_request
    rv = self.dispatch_request()
  File "/opt/indico/.venv/lib/python2.7/site-packages/flask/app.py", line 1598, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/opt/indico/.venv/lib/python2.7/site-packages/indico/web/flask/util.py", line 114, in wrapper
    return obj().process()
  File "/opt/indico/.venv/lib/python2.7/site-packages/indico/web/rh.py", line 270, in process
    res = self._do_process()
  File "/opt/indico/.venv/lib/python2.7/site-packages/indico/web/rh.py", line 249, in _do_process
    return self._process()
  File "/opt/indico/.venv/lib/python2.7/site-packages/indico/modules/auth/controllers.py", line 97, in _process
    response = multipass.handle_login_form(provider, form.data)
  File "/opt/indico/.venv/lib/python2.7/site-packages/flask_multipass/core.py", line 459, in handle_login_form
    response = provider.process_local_login(data)
  File "/opt/indico/.venv/lib/python2.7/site-packages/flask_multipass/providers/sqlalchemy.py", line 55, in process_local_login
    type(self).identifier_column == data['identifier']).first()
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/orm/query.py", line 2755, in first
    ret = list(self[0:1])
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/orm/query.py", line 2547, in __getitem__
    return list(res)
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/orm/query.py", line 2855, in __iter__
    return self._execute_and_instances(context)
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/orm/query.py", line 2878, in _execute_and_instances
    result = conn.execute(querycontext.statement, self._params)
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 945, in execute
    return meth(self, multiparams, params)
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/sql/elements.py", line 263, in _execute_on_connection
    return connection._execute_clauseelement(self, multiparams, params)
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1053, in _execute_clauseelement
    compiled_sql, distilled_params
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1189, in _execute_context
    context)
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1405, in _handle_dbapi_exception
    util.reraise(*exc_info)
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1182, in _execute_context
    context)
  File "/opt/indico/.venv/lib/python2.7/site-packages/sqlalchemy/engine/default.py", line 470, in do_execute
    cursor.execute(statement, parameters)
ValueError: A string literal cannot contain NUL (0x00) characters.

{u'data': {u'get': {},
           u'headers': {'Accept': u'image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, image/png, */*',
                        'Accept-Charset': u'iso-8859-1,utf-8;q=0.9,*;q=0.1',
                        'Accept-Language': u'en',
                        'Connection': u'Keep-Alive',
                        'Content-Length': u'181',
                        'Content-Type': u'application/x-www-form-urlencoded',
                        'Cookie': u'indico_session=684c10fc-9978-4f25-9421-e194202add82',
                        'Host': u'indico.nbi.ku.dk',
                        'Pragma': u'no-cache',
                        'User-Agent': u'Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 5.1; Trident/4.0)'},
           u'json': None,
           u'post': {'_provider': u'indico',
                     'csrf_token': u'00000000-0000-0000-0000-000000000000',
                     'identifier': [u'../../../../../../../../etc/passwd\x00',
                                    u''],
                     'next': u'%2F%23create-event%3Alecture',
                     'password': u'<6 chars hidden>'},
           u'url': {}},
 u'endpoint': u'auth.login',
 u'id': 'd4a07653228b45d5',
 u'ip': '<redacted>',
 u'method': 'POST',
 u'referrer': None,
 u'rh': 'RHLogin',
 u'time': '2017-11-06T07:45:38.031777',
 u'url': u'https://indico.nbi.ku.dk/login/',
 u'user': None,
 u'user_agent': u'Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 5.1; `Trident/4.0)'}

2017-11-06 07:10:19,267  1ee44ec4ea2c40d4  indico.flask - ERROR errors.py:101 -- must be string without null bytes, not unicode

Traceback (most recent call last):
  File "/opt/indico/.venv/lib/python2.7/site-packages/flask/app.py", line 1612, in full_dispatch_request
    rv = self.dispatch_request()
  File "/opt/indico/.venv/lib/python2.7/site-packages/flask/app.py", line 1598, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/opt/indico/.venv/lib/python2.7/site-packages/indico/web/flask/util.py", line 114, in wrapper
    return obj().process()
  File "/opt/indico/.venv/lib/python2.7/site-packages/indico/web/rh.py", line 270, in process
    res = self._do_process()
  File "/opt/indico/.venv/lib/python2.7/site-packages/indico/web/rh.py", line 229, in _do_process
    args_result = self._process_args()
  File "/opt/indico/.venv/lib/python2.7/site-packages/indico/modules/categories/controllers/display.py", line 447, in _process_args
    date = datetime.strptime(request.args['date'], '%Y-%m-%d')
TypeError: must be string without null bytes, not unicode

{u'data': {u'get': {'date': u'\x00ceccfp'},
           u'headers': {'Accept': u'image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, image/png, */*',
                        'Accept-Charset': u'iso-8859-1,utf-8;q=0.9,*;q=0.1',
                        'Accept-Language': u'en',
                        'Connection': u'Keep-Alive',
                        'Host': u'indico.nbi.ku.dk',
                        'Pragma': u'no-cache',
                        'User-Agent': u'Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 5.1; Trident/4.0)'},
           u'json': None,
           u'post': {},
           u'url': {'category_id': 8}},
 u'endpoint': u'categories.overview',
 u'id': '1ee44ec4ea2c40d4',
 u'ip': '<redacted>',
 u'method': 'GET',
 u'referrer': None,
 u'rh': 'RHCategoryOverview',
 u'time': '2017-11-06T07:10:19.290345',
 u'url': u'https://indico.nbi.ku.dk/category/8/overview?date=\x00ceccfp',
 u'user': None,
 u'user_agent': u'Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 5.1; Trident/4.0)'}

[ThiefMaster]

While this error is not pretty, it is completely harmless. Creating noise in the log file is the only undesirable thing that happens in such a case.

FYI, SQLAlchemy properly passes user input as query parameters instead of interpolating it, so there is no need for any kind of escaping there.

The only workaround to avoid this kind of noise I can see is failing e.g. a 400 error if there's any query string value or url segment containing a NUL byte.

[chrulle]

If it only created noise in the log file that would be ok. The problem is that it is not correctly classified. It should trigger an error message to the user. Instead it triggers a python process "crash" because of an unhandled exception. This triggers an "error" level message to the logs and an email to the admin. In case someone is trying a brute force attack (or a security scan) it causes 1000+ mails to go to the admin. That is not harmless. I cannot turn off the sending of emails, since I actually want to see real errors.

[ThiefMaster]

Like I said in my previous comment, I think rejecting requests with NULs in query string arguments makes sense and this should cover most of the noise such security scans generate. If you have any cases where other nonsense data caused exceptions please let us know!

---

I cannot turn off the sending of emails, since I actually want to see real errors.

Error emails are annoying in general - we got quite some noise in the past too (especially when we were still using legacy indico). You might want to consider using Sentry and completely disable error emails. In Sentry you can easily mark specific errors as ignored.

Also, for real errors encountered by users they always have the option to send an error report which would send you an email even if the email logger for errors is disabled.