Skip to content
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

http-shellshock: Fix false positive by matching at start of line #2089

Closed
wants to merge 1 commit into from

Conversation

@andersk
Copy link

andersk commented Jul 28, 2020

Running the http-shellshock script on a Zulip installation results in a false positive because Zulip (intentionally and safely) reflects the HTTP User-Agent header as the data-platform attribute on a <div> so that it can be matched with CSS rules.

If an actually vulnerable server were really running our payload

() { :;}; echo; echo "therandomstring"

then the extra echo would cause therandomstring to appear at the start of a line. So we can avoid the false positive by only triggering on matches at the start of a line.

@andersk andersk force-pushed the andersk:shellshock-false-positive branch from 4f82ac2 to ae3847a Jul 28, 2020
@nnposter
Copy link

nnposter commented Jul 28, 2020

I am guessing that this might not be a completely safe assumption. If bash is not the process invoked directly by the web server but it is somewhere deeper in the process tree that produces only a portion of the output then it is not assured that the final response will have the random mark at the beginning of a line.

Perhaps a more robust approach to differentiate between a vulnerable shell invocation and a request reflection would be to verify that the mark is not preceded by "echo", accounting for a few possible space encodings. Something along these lines:

local function find_mark (str, mark)
  local spaces = {" ", "&nbsp;", "&#32;", "+", "%20"}
  local pos = 1
  while true do
    pos = str:find(mark, pos, true)
    if not pos then break end
    local found = true
    for _, space in ipairs(spaces) do
      local cmd = "echo" .. space
      local cmdpos = pos - #cmd
      if cmdpos > 0 and str:find(cmd, cmdpos, true) == cmdpos then
        -- just a reflected request
        found = false
        break
      end
    end
    if found then return true end
    pos = pos + #mark
  end
  return false
end

This gives you the following:

> find_mark("acb222zzz", "222")
true
> find_mark("acbecho222zzz", "222")
true
> find_mark("acbecho+222zzz", "222")
false
> find_mark("acbecho+222zzz222", "222")
true
@andersk
Copy link
Author

andersk commented Jul 29, 2020

If bash is not the process invoked directly by the web server but it is somewhere deeper in the process tree that produces only a portion of the output then it is not assured that the final response will have the random mark at the beginning of a line.

You misunderstand. We know the mark will be at the beginning of a line not because of an assumption that the bash payload is the first thing to execute, but because the bash payload begins with an empty echo; statement that prints a newline before a second echo statement prints the mark.

@nnposter
Copy link

nnposter commented Jul 29, 2020

I understand fine. There is just no assurance that the newlines will be preserved under these circumstances.

@andersk andersk force-pushed the andersk:shellshock-false-positive branch from ae3847a to 1bd3cde Jul 29, 2020
@andersk
Copy link
Author

andersk commented Jul 29, 2020

If you’re worried about an endpoint that executes a bash script in the middle and reads its output and writes something else, there’s no assurance that anything will be preserved.

In any event, what do you think about this alternate solution of inserting extra quotes into the payload that are removed by bash?

() { :;}; echo; echo "t""herandomstring"
@nnposter
Copy link

nnposter commented Jul 29, 2020

This would certainly accomplish the same objective, The only downside is that I was contemplating to remove the currently unnecessary quotes from the payload to reduce the chance of interfering with how the downstream process command line gets composed (possibly with quotes).

How about this instead? echo; echo -n random1; echo random2

$ (echo; echo -n abc; echo def) | xxd
00000000: 0a61 6263 6465 660a                      .abcdef.
Running the http-shellshock script on a Zulip installation results in
a false positive because Zulip (intentionally and safely) reflects the
HTTP User-Agent header as the data-platform attribute on a <div> so
that it can be matched with CSS rules.

https://github.com/zulip/zulip/blob/3.0/templates/zerver/portico.html#L11

Change the payload to write the random string with multiple echo
statements, so that the random string won’t match a reflected payload
that was not actually executed.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk andersk force-pushed the andersk:shellshock-false-positive branch from 1bd3cde to 6242abe Jul 29, 2020
@andersk
Copy link
Author

andersk commented Jul 29, 2020

Okay. Updated as such.

@nnposter nnposter self-assigned this Jul 29, 2020
@nnposter
Copy link

nnposter commented Jul 29, 2020

The fix has been committed as r37969, Thank you for reporting the issue and coming up with a solution!

@nmap-bot nmap-bot closed this in f278aca Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.