Fix reflected XSS attack when hitting getCookie endpoint

[SilleBille]

This patch sanitizes the Server generated error message in getCookie, to escape
HTML tags, if present.

Resolves: BZ#1789907 CVE-2019-10221

Signed-off-by: Dinesh Prasanth M K <dmoluguw@redhat.com>

[edewata]

IIUC GetCookie servlet will generate a JavaScript code below that will construct a JavaScript header object from the Java header object (see CMSTemplate.renderOutput()):

out.println("header." + n + " = " + renderValue(v) + ";");

In renderValue() the values are escaped for JavaScript using escapeJavaScriptString().

The servlet will then insert the code into one of the templates below and send the page to the client (see web.xml):

• /admin/ca/sendCookie.template
• /admin/ca/securitydomainlogin.template

When the client renders the page it will use the JavaScript header object like this:

document.write('<form name="cookieForm" method="post" action="'+result.header.url+'">');

I think ideally the escaping for HTML should happen just before it's used. It can be done using escapeHtml():

document.write(
    '<form name="cookieForm" method="post" action="' +
    escapeHtml(result.header.url) +
    '">');

or using jQuery which should escape the value automatically:

$('form[name="cookieForm"]').attr('action', result.header.url)

If the value is escaped too early on the server side there's a risk that the value might not be rendered correctly since the client might use the value in different ways. However, since this is a legacy servlet which will be replaced pretty soon, feel free to merge but just be aware of the potential issue.

[cipherboy]

I think ideally the escaping for HTML should happen just before it's used.

Wouldn't it be safer to transfer it as e.g., a JSON object, and then do:

document.write('<form name="cookieForm" method="post">');
data = JSON.parse(ret);
document.getElementByName("cookieForm").action = data["url"];

(Or, replace data with how it is done currently? Then the browser can set the value directly as the action instead of having to escape it (and perhaps, we just need to deny the javascript: src prefix).

[SilleBille]

thanks for the review Endi and Alex.

Endi, I did verify the param values using a dummy url:

<host>/ca/admin/ca/getCookie?url=https://redhat.com?test=<script>alert("test")</script>

var header = new Object();
var fixed = new Object();
var recordSet = new Array;
var result = new Object();
var httpParamsCount = 0;
var httpHeadersCount = 0;
var authTokenCount = 0;
var serverAttrsCount = 0;
header.HTTP_PARAMS = new Array;
header.HTTP_HEADERS = new Array;
header.AUTH_TOKEN = new Array;
header.SERVER_ATTRS = new Array;
header.sd_pwd = null;
header.url = "https://redhat.com?test=<script>alert("test")</script>";
header.sdname = "EXAMPLE";
header.errorString = "Failed Authentication";
header.host = "redhat.com";
header.subsystem = null;
header.sd_uid = null;
header.sdhost = "BLEH";
result.header = header;
result.fixed = fixed;
result.recordSet = recordSet;

I'll merge this PR now based on the ACKs and green light from CI

[edewata]

When we convert this servlet into REST API later we'll definitely be using JSON, but I don't know if we want to make major changes in the old servlets right now. If you think the JSON code can be reused in the REST API later feel free to do so.

[cipherboy]

@SilleBille Did you test this with a URL like:

https://pki.example.com/ca/ee/ca/somePage?param=value&otherParam=otherValue

and did it behave correctly?

[SilleBille]

@SilleBille Did you test this with a URL like:

https://pki.example.com/ca/ee/ca/somePage?param=value&otherParam=otherValue

and did it behave correctly?

hmmm. I received the following:

<SCRIPT LANGUAGE="JavaScript">
var header = new Object();
var fixed = new Object();
var recordSet = new Array;
var result = new Object();
var httpParamsCount = 0;
var httpHeadersCount = 0;
var authTokenCount = 0;
var serverAttrsCount = 0;
header.HTTP_PARAMS = new Array;
header.HTTP_HEADERS = new Array;
header.AUTH_TOKEN = new Array;
header.SERVER_ATTRS = new Array;
header.sd_pwd = null;
header.url = "https://pki.example.com/ca/ee/ca/somePage?param=value";
header.sdname = "EXAMPLE";
header.errorString = "Failed Authentication";
header.host = "pki.example.com";
header.subsystem = null;
header.sd_uid = null;
header.sdhost = "mkd.fedora.office";
result.header = header;
result.fixed = fixed;
result.recordSet = recordSet;
</SCRIPT>

See that the &otherParam is missing. However, I observed the same behavior before this patch too. So that shouldn't be a problem I believe

[cipherboy]

Hmmm ok, weird. shrugs :)