Add script http-cookie-flags.nse: Report insecurely set HTTP session cookie flags. #669
Conversation
|
|
||
| --- | ||
| -- @usage | ||
| -- nmap -p 443 --script ssl-cert-intaddr <target> |
There was a problem hiding this comment.
copy-paste error, wrong script name
| @@ -0,0 +1,139 @@ | |||
|
|
|||
There was a problem hiding this comment.
Need local string = require "string" here. Helpful code checking scripts are available on our Code Standards wiki page.
There was a problem hiding this comment.
I will fix that and check out those scripts.
| action = function(host, port) | ||
| local all_issues = stdnse.output_table() | ||
|
|
||
| all_issues['/'] = check_path(host, port, '/') |
There was a problem hiding this comment.
Most of our http-* scripts have a uri or path script-arg to allow requesting a specific path other than /. Please modify this to accept the same. It may be useful to let the user pass a specific cookie name to check, too.
| '^FedAuth$', | ||
| '^ASPXAUTH$', | ||
| '[Ss][Ee][Ss][Ss][Ii][Oo][Nn]_*[Ii][Dd]' | ||
| } |
There was a problem hiding this comment.
A couple more patterns: ^session$ is used by some frameworks, and your SESSION_ID match could use any non-alpha character in place of the underscore: [^%a]*
There was a problem hiding this comment.
I will add those & change session_id.
| without the httponly flag. Reports any session cookies set over SSL without | ||
| the secure flag. If http-enum.nse is also run, any interesting paths found | ||
| by it will be checked in addition to the root. | ||
| ]] |
There was a problem hiding this comment.
I wonder if this would be better called simply "http-cookie-flags" and have an option to check flags on all cookies. By removing "session" from the name, we open it up to other patterns (in the default case) that could specify other sensitive cookie names, even if they are not session cookies. Of course, the description would have to make it clear that only known-sensitive cookies are being reported.
There was a problem hiding this comment.
That makes sense. I will rename the script.
| local all_pages = stdnse.registry_get({host.ip, 'www', port.number, 'all_pages'}) | ||
| if all_pages then | ||
| for _,path in ipairs(all_pages) do | ||
| all_issues[path] = check_path(host, port, path) |
There was a problem hiding this comment.
I worry that this might result in a lot of duplicate output if a cookie is set with a path of / for all lower-level urls. We should be able to pass the output table to the check_path function and have it add discovered cookies to the table based on the cookie's path attribute instead of simply using the path that was requested.
There was a problem hiding this comment.
That's true. I'll change the structure of the output table like you suggest
However, in some cases I think that could lead to confusing output if different web apps set different cookies for the same path. For example:
GET /mail HTTP/1.0
200 OK
Set-Cookie: session_id=12345; path=/
and:
GET /docs HTTP/1.0
200 OK
Set-Cookie: ASP.NET_SessionId=67890; path=/
The script's output would be:
| /:
| session_id:
| httponly flag not set
| ASP.NET_SessionId:
| httponly flag not set
What do you think about adding the requested path (from the first request the cookie was seen from, anyway) as an additional information item with the cookie, like this:
| /:
| session_id:
| httponly flag not set
| Initially set from /mail
| ASP.NET_SessionId:
| httponly flag not set
| Initially set from /docs
…o check flags on. Changed is_session_cookie() into a closure implementing desired session cookie name test.
Examines cookies set by HTTP services. Reports any session cookies set without the httponly flag. Reports any session cookies set over SSL without the secure flag. If http-enum.nse is also run, any interesting paths found by it will be checked in addition to the root.