Summary: | Authority lookup in advanced editor overencodes HTML | ||
---|---|---|---|
Product: | Koha | Reporter: | Phil Ringnalda <phil> |
Component: | Cataloging | Assignee: | Phil Ringnalda <phil> |
Status: | CLOSED FIXED | QA Contact: | Katrin Fischer <katrin.fischer> |
Severity: | normal | ||
Priority: | P5 - low | CC: | arthur.suzuki, jonathan.druart, lucas, m.de.rooy, wainuiwitikapark |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
23.05.00,22.11.04,22.05.12,21.11.20
|
|
Circulation function: | |||
Bug Depends on: | 26102 | ||
Bug Blocks: | |||
Attachments: |
Bug 33144: Unescape text from authority lookup for advanced editor
Bug 33144: Unescape text from authority lookup for advanced editor Bug 33144: Unescape text from authority lookup for advanced editor |
Description
Phil Ringnalda
2023-03-06 03:11:15 UTC
Created attachment 147776 [details] [review] Bug 33144: Unescape text from authority lookup for advanced editor While the basic editor is happy with an array of subfields it can stuff into separate fields, the advanced editor needs to get a JS string back from the authority lookup plugin, because it is going to just add the whole thing as text. The string has to be HTML entity encoded, both to not allow XSS and just to not break the window, but it needs to then be unencoded before being inserted into the editor. Test plan: 1. Set the system preference EnableAdvancedCatalogingEditor to Enable 2. Edit any Topical Term authority, and at the end of tag 150 subfield a, add & </script> 3. Cataloging - Advanced editor 4. Press return in the editor to get a new blank line, type 650 and press tab three times, then type Ctrl-Shift-L 5. Search for your modified authority, and click Choose 6. Verify that the tiny popup opened by the search window finished its job and closed itself 7. Verify that your 650 now shows as "‡aAbduction & </script>‡vDrama" rather than "‡aAbduction & </script>‡vDrama." I'm no expert in assessing the XSS risks of code copy-pasted from Stack Overflow, but I've been reading Wladimir's writing on security and running his code for years, so I'd trust https://stackoverflow.com/questions/1912501/unescape-html-entities-in-javascript/34064434#34064434 and in fact we already do that same thing in https://git.koha-community.org/Koha-community/Koha/src/commit/fe872b792037ee2f8cce0d25f95c1e4f739ffe49/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt#L820 Created attachment 147817 [details] [review] Bug 33144: Unescape text from authority lookup for advanced editor While the basic editor is happy with an array of subfields it can stuff into separate fields, the advanced editor needs to get a JS string back from the authority lookup plugin, because it is going to just add the whole thing as text. The string has to be HTML entity encoded, both to not allow XSS and just to not break the window, but it needs to then be unencoded before being inserted into the editor. Test plan: 1. Set the system preference EnableAdvancedCatalogingEditor to Enable 2. Edit any Topical Term authority, and at the end of tag 150 subfield a, add & </script> 3. Cataloging - Advanced editor 4. Press return in the editor to get a new blank line, type 650 and press tab three times, then type Ctrl-Shift-L 5. Search for your modified authority, and click Choose 6. Verify that the tiny popup opened by the search window finished its job and closed itself 7. Verify that your 650 now shows as "‡aAbduction & </script>‡vDrama" rather than "‡aAbduction & </script>‡vDrama." Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 148038 [details] [review] Bug 33144: Unescape text from authority lookup for advanced editor While the basic editor is happy with an array of subfields it can stuff into separate fields, the advanced editor needs to get a JS string back from the authority lookup plugin, because it is going to just add the whole thing as text. The string has to be HTML entity encoded, both to not allow XSS and just to not break the window, but it needs to then be unencoded before being inserted into the editor. Test plan: 1. Set the system preference EnableAdvancedCatalogingEditor to Enable 2. Edit any Topical Term authority, and at the end of tag 150 subfield a, add & </script> 3. Cataloging - Advanced editor 4. Press return in the editor to get a new blank line, type 650 and press tab three times, then type Ctrl-Shift-L 5. Search for your modified authority, and click Choose 6. Verify that the tiny popup opened by the search window finished its job and closed itself 7. Verify that your 650 now shows as "‡aAbduction & </script>‡vDrama" rather than "‡aAbduction & </script>‡vDrama." Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master for 23.05. Nice work everyone, thanks! Nice work, thanks everyone! Pushed to 22.11.x for the next release. Backported to 22.05.x for upcoming 22.05.12 applied to 21.11.x for 21.11.20 Not backported to 21.05.x |