New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Space in unquoted cookie value #844

Closed
nnposter opened this Issue Apr 14, 2017 · 0 comments

Comments

Projects
None yet
1 participant
@nnposter

nnposter commented Apr 14, 2017

As of now, function parse_set_cookie in http.lua largely follows RFC 2109 when parsing header Set-Cookie.

One real-life case that is not parsed properly is when the unquoted cookie value contains a space. This case is not valid under RFC 2109 but it is valid under RFC 6265, Section 5.2.

The following cookie is sent by a Huawei device. Note the space between the brand and the model.

Set-Cookie: SESSIONID=IgAAABjN8b3(removed)NsLRIiSpHLPn1lE=&IgAAA(removed)MT6Bw==&Huawei USG6320&langfrombrows=en-US&copyright=2014;secure

I am proposing to update the function to parse the cookie name/value pair in accordance with RFC 6265 (but still support quoted values) as follows:

* Allows unquoted cookie values to contain whitespace, as defined in RFC6265.
--- a/nselib/http.lua
+++ b/nselib/http.lua
@@ -725,9 +725,8 @@
 --
 -- Every key except "name" and "value" is optional.
 --
--- This function attempts to support the cookie syntax defined in RFC 2109
--- along with the backwards-compatibility suggestions from its section 10,
--- "HISTORICAL". Values need not be quoted, but if they start with a quote they
+-- This function attempts to support the header parser defined in RFC 6265,
+-- Section 5.2. Values need not be quoted, but if they start with a quote they
 -- will be interpreted as a quoted string.
 parse_set_cookie = function (s)
   local name, value
@@ -736,27 +735,22 @@
   local cookie = {}
 
   -- Get the NAME=VALUE part.
-  local pos = skip_space(s, 1)
-  pos, cookie.name = get_token(s, pos)
-  if not cookie.name then
+  local pos
+  _, pos, cookie.name = s:find("^[ \t]*(.-)[ \t]*=[ \t]*")
+  if not (cookie.name or ""):find("^[^;]+$") then
     return nil, "Can't get cookie name."
   end
-  pos = skip_space(s, pos)
-  if s:sub(pos, pos) ~= "=" then
-    return nil, string.format("Expected '=' after cookie name \"%s\".", cookie.name)
-  end
   pos = pos + 1
-  pos = skip_space(s, pos)
   if s:sub(pos, pos) == "\"" then
     pos, cookie.value = get_quoted_string(s, pos)
+    if not cookie.value then
+      return nil, string.format("Can't get value of cookie named \"%s\".", cookie.name)
+    end
+    pos = skip_space(s, pos)
   else
-    _, pos, cookie.value = s:find("([^; \t]*)", pos)
+    _, pos, cookie.value = s:find("^(.-)[ \t]*%f[;\0]", pos)
     pos = pos + 1
   end
-  if not cookie.value then
-    return nil, string.format("Can't get value of cookie named \"%s\".", cookie.name)
-  end
-  pos = skip_space(s, pos)
 
   -- Loop over the attributes.
   while s:sub(pos, pos) == ";" do

Please let me know if you have any questions or concerns. Otherwise I will commit the patch in a few weeks.

nmap-bot pushed a commit that referenced this issue Apr 27, 2017

@nmap-bot nmap-bot closed this in 0b36ba5 Apr 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment