search

github / secure_headers

Manages application of security headers with many safe defaults
MIT License
3.16k stars 252 forks source link

fix: Avoid throwing cookie headers when encountering an empty cookie-av #516

Closed MrLukeSmith closed 3 months ago

MrLukeSmith commented 3 months ago

Background:

We ran into an issue with a trusted client sending Set-Cookie headers which were invalid and which we were proxying to the end user.

The headers were in the form: Set-Cookie: name=value; Max-Age=315360000; domain=example.com; path=/; SameSite=Strict;; secure

When SecureHeaders::Cookie#parse encounters the ;; it blows up because:

      cookie.split(/[;,]\s?/).each do |pairs| # pairs == ""
        name, values = pairs.split("=", 2)    # pairs.split() => []
                                              # ie. name == values == nil
        name = CGI.unescape(name)             # raises exception

ie.

$ for rb in 2.0 2.1 2.2 2.3 2.4 2.5 2.6; do chruby-exec $rb -- ruby -r cgi -e 'p [RUBY_VERSION, "".split(/=/,2)]; p CGI.unescape(nil)'; done

["2.0.0", []]
/opt/rubies/2.0.0-p648/lib/ruby/2.0.0/cgi/util.rb:17:in `unescape': undefined method `tr' for nil:NilClass (NoMethodError)
        from -e:1:in `<main>'
["2.1.10", []]
/opt/rubies/ruby-2.1.10/lib/ruby/2.1.0/cgi/util.rb:18:in `unescape': undefined method `tr' for nil:NilClass (NoMethodError)
        from -e:1:in `<main>'
["2.2.6", []]
/opt/rubies/ruby-2.2.6/lib/ruby/2.2.0/cgi/util.rb:18:in `unescape': undefined method `tr' for nil:NilClass (NoMethodError)
        from -e:1:in `<main>'
["2.3.6", []]
/opt/rubies/ruby-2.3.6/lib/ruby/2.3.0/cgi/util.rb:19:in `unescape': undefined method `tr' for nil:NilClass (NoMethodError)
        from -e:1:in `<main>'
["2.4.0", []]
-e:1:in `unescape': no implicit conversion of nil into String (TypeError)
        from -e:1:in `<main>'
["2.5.0", []]
Traceback (most recent call last):
        1: from -e:1:in `<main>'
-e:1:in `unescape': no implicit conversion of nil into String (TypeError)
["2.6.5", []]
Traceback (most recent call last):
        1: from -e:1:in `<main>'
-e:1:in `unescape': no implicit conversion of nil into String (TypeError)

Solution:

Allow the following header to be parsed without throwing an exception:

Set-Cookie: name=value; Max-Age=315360000; domain=example.com; path=/; SameSite=Strict;; secure

It's not valid - https://tools.ietf.org/html/rfc6265#section-4.1.1 - but it shouldn't result in a nil exception.

MrLukeSmith commented 3 months ago

I was pointed at the original request from back in the day, which I missed when searching. It looks like an alternative solution was preferred, closing this in lieu of that.