escapeJs vulnerable to XSS#90
Conversation
|
Hey @jwnasambu, Can you please review it. Any suggestion are welcome. |
| public void escapeJs_shouldPreventXSSViaBackslashInjection() { | ||
| // exact payload from the bug report | ||
| String result = ui.escapeJs("Foo \\\"}];alert(0);[// Bar"); | ||
| Assert.assertTrue(result.contains("\\\\")); // backslash must be doubled |
There was a problem hiding this comment.
Kindly asserting only contains("\\"); is too weak and could pass even if the escaping regresses.
so please can either assert the exact expected escaped output for the full payload using Assert.assertEquals(expected, ui.escapeJs(input)), or
assert stronger conditions example that quotes are escaped and that the unescaped breakout sequence .
There was a problem hiding this comment.
Hey @jwnasambu , thanks for reviewing it. Yeah i have added stricter assertion and you can now review that too.
Co-authored-by: jwnasambu <33891016+jwnasambu@users.noreply.github.com>
|
Hey @jwnasambu , Can you please review it? |
|
Hey @jwnasambu, can you please approve its workflow? |
|
Hey @ibacher and @jwnasambu can you please merge this PR or otherwise tell me for any suggestions to resolve this issue/bug? |
|
Hey @ibacher and @jwnasambu can you please merge this PR or tell me for any suggestions to resolve this issue/bug? |
2 participants
Issue I worked on
https://openmrs.atlassian.net/browse/RA-1424
Summary
The escapeJs method in UiUtils.java did not escape backslashes, making it vulnerable to XSS. A crafted input like Foo "}];alert(0);[// injected into a JS string context (e.g., the breadcrumb in editSection.gsp) could break out of the string and execute arbitrary JavaScript.
Fix
Note: The vulnerable rendering surface is editSelection.gsp in openmrs-module-registrationapp, which calls ui.escapeJs(ui.format(patient)). No change is needed there — this fix protects it automatically.