UKGovLD / registry-core

Linked data registry - core application and example UI
https://github.com/UKGovLD/registry-core/wiki
Apache License 2.0
23 stars 9 forks source link

XSS querystring parameter input values not escaped before output to HTML #181

Closed mike-gormley closed 1 year ago

mike-gormley commented 1 year ago

Issue been raised by a member of public to our security team. Cross-site scripting vulnerability via query-string parameters input value being outputted in the HTML without sanitation.

Example:

1 - Visit this uri :- /ui/login?return=javascript:alert("hello world"); 2 - The value of parameter 'return' will reflect in the response in the href attribute 3 – If a user clicks "Close", JavaScript code will be executed

Here's another example: /ui/actions/create-redirect-page?entity=%22%3E%3Cimg%20src=x%20onerror=prompt(4)%3E

We completed some initial investigation into the matter,,,

HTML output of first example on login velocity template in registry-config-base repo: https://github.com/UKGovLD/registry-config-base/blob/master/ldregistry/templates/login.vm#L137

Input appeared to be cleaned earlier in the same file: https://github.com/UKGovLD/registry-config-base/blob/master/ldregistry/templates/login.vm#L17

xssCleanUri defined in registry-core only encodes characters for HTML output but doesn't prevent the execution of JavaScript: https://github.com/UKGovLD/registry-core/blob/07520783cdd34ef70067b41da907ae70aeb06f39/src/main/java/com/epimorphics/registry/webapi/LibReg.java#L95

Running registry-core version 2.3.12.

der commented 1 year ago

Thanks for the report, agree with the diagnosis, we will get xssCleanUri improved. If you need a hot fix in the meantime then use a regex (replace()) on L17 to trap javascript; and sanitise.

mike-gormley commented 1 year ago

@der Thank you for the prompt response as well as a resolution in quick time. I have deployed version 2.3.14 in our testing environment and that has addressed the above concern for executing JavaScript.

der commented 1 year ago

That was quick! Was just messaging you to ask you to check. Glad it works.