ValueError when If-Modified-Since is invalid for StaticFileHandler request

[rgajrawala]

Hello, recently our application underwent a security scan and I noticed our Tornado app was returning HTTP 500 for some requests:

HTTPServerRequest(protocol='http', host='ip-x-x-x-x.ec2.internal', method='GET', uri='/', version='HTTP/1.1', remote_ip='x.x.x.x')
Traceback (most recent call last):
  File "/opt/service/.local/lib/python3.11/site-packages/tornado/web.py", line 1790, in _execute
    result = await result
             ^^^^^^^^^^^^
  File "/opt/service/.local/lib/python3.11/site-packages/tornado/web.py", line 2695, in get
    if self.should_return_304():
       ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/service/.local/lib/python3.11/site-packages/tornado/web.py", line 2820, in should_return_304
    if_since = email.utils.parsedate_to_datetime(ims_value)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/email/utils.py", line 200, in parsedate_to_datetime
    raise ValueError('Invalid date value or format "%s"' % str(data))
ValueError: Invalid date value or format "${jndi:ldap://log4shell-generic-xxx${lower:ten}.w.nessus.org/nessus}"
2024-06-26 11:46:16.900 500 GET / (x.x.x.x) 8.71ms

Looks like tornado.web.StaticFileHandler.should_return_304 is blindly parsing the If-Modified-Since header resulting in 500s. Ideally, invalid headers would return 400s.

[mcg1969]

Just discovered this as well! I'm looking for a workaround. Will comment again if I find one

[mcg1969]

It's possible this is being trigged by a change in browser behavior. I am finding If-Modified-Since: 0 in the request headers that trigger the error.

EDIT: Turns out that this has been a feature of our application for years—it's a cache buster—but we weren't passing these headers to tornado in the past.

[mcg1969]

I agree that it should not be returning a 500 in this case. I would suggest the proper response is to return False if the headers do not parse properly. With that in mind, here is perhaps a better workaround:

class MyStaticFileHandler(tornado.web.StaticFileHandler):
    def should_return_304(self):
        try:
            return super().should_return_304()
        except Exception:
            return False

[bdarnell]

Looks like @mcg1969 is correct, and invalid If-Modified-Since headers should be ignored instead of returning a 5xx or 4xx error. RFC 9110 section 13.1.3 says "A recipient MUST ignore the If-Modified-Since header field if the received field value is not a valid HTTP-date".

[mcg1969]

Just coming over here to share that link! Thanks @bdarnell

[rgajrawala]

@bdarnell I made a PR to fix this since we're unable to inject our own StaticFileHandler into the third-party library we're using. https://github.com/tornadoweb/tornado/pull/3412

[bdarnell]

Fixed in #3412.