Bug 25381 - XSLTs should not define entities
Summary: XSLTs should not define entities
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Didier Gautheron
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2020-05-05 23:25 UTC by David Cook
Modified: 2021-12-13 21:08 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.03,20.05.09


Attachments
Bug 25381: XSLTs should not define entities (34.49 KB, patch)
2020-05-08 14:07 UTC, Didier Gautheron
Details | Diff | Splinter Review
Bug 25381: XSLTs should not define entities (34.49 KB, patch)
2021-01-21 22:46 UTC, Eden
Details | Diff | Splinter Review
Bug 25381: XSLTs should not define entities (34.54 KB, patch)
2021-01-21 22:49 UTC, Eden
Details | Diff | Splinter Review
Bug 25381: XSLTs should not define entities (34.60 KB, patch)
2021-01-21 23:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25381: XSLTs should not define entities (34.65 KB, patch)
2021-01-21 23:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25381: (QA follow-up) Fix occurence in the one file that escaped (966 bytes, patch)
2021-01-21 23:45 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2020-05-05 23:25:39 UTC
Bug 23290 explicitly disabled entity expansion during XML parsing for security reasons.

However, many XSLTs define the following entity:
<!ENTITY nbsp "&#160;" >

They don't use the entity &nbsp, but its presence could lead to confusion (as noted in Bug 25373).

These entities should be removed from the XSLTs.
Comment 1 Didier Gautheron 2020-05-08 14:07:13 UTC
Created attachment 104599 [details] [review]
Bug 25381: XSLTs should not define entities

Bug 23290 explicitly disabled entity expansion during XML parsing for security reasons.

However, many XSLTs define the following entity:
<!ENTITY nbsp "&#160;" >

They don't use the entity &nbsp, but its presence could lead to confusion.
Comment 2 Didier Gautheron 2020-05-08 14:09:51 UTC
Output of:

for i in $(git grep -lF '<!DOCTYPE stylesheet [<!ENTITY nbsp "&#160;" >]>' ) ; do sed -i 's/ \[<!ENTITY nbsp "\&#160;" >\]//' $i ; done

In Koha source tree
Comment 3 Aleisha Amohia 2021-01-20 23:52:33 UTC
Is this ready for testing? If yes can you please adjust the bug status accordingly?
Comment 4 Katrin Fischer 2021-01-21 19:53:48 UTC
I have tested the patch appleis still, switching status.
Comment 5 Aleisha Amohia 2021-01-21 21:36:15 UTC
Cool! Test plan would be to simply eyeball the patch and check that the entity definition has been removed correctly and the code still looks OK!
Comment 6 Eden 2021-01-21 22:46:49 UTC
Created attachment 115632 [details] [review]
Bug 25381: XSLTs should not define entities

Bug 23290 explicitly disabled entity expansion during XML parsing for security reasons.

However, many XSLTs define the following entity:
<!ENTITY nbsp "&#160;" >

They don't use the entity &nbsp, but its presence could lead to confusion.
Comment 7 Eden 2021-01-21 22:49:55 UTC
Created attachment 115635 [details] [review]
Bug 25381: XSLTs should not define entities

Bug 23290 explicitly disabled entity expansion during XML parsing for security reasons.

However, many XSLTs define the following entity:
<!ENTITY nbsp "&#160;" >

They don't use the entity &nbsp, but its presence could lead to confusion.

Signed-off-by: Eden Bacani <eden.bacani@gmail.com>
Comment 8 Katrin Fischer 2021-01-21 23:43:15 UTC
Created attachment 115650 [details] [review]
Bug 25381: XSLTs should not define entities

Bug 23290 explicitly disabled entity expansion during XML parsing for security reasons.

However, many XSLTs define the following entity:
<!ENTITY nbsp "&#160;" >

They don't use the entity &nbsp, but its presence could lead to confusion.

Signed-off-by: Eden Bacani <eden.bacani@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2021-01-21 23:45:10 UTC
Created attachment 115651 [details] [review]
Bug 25381: XSLTs should not define entities

Bug 23290 explicitly disabled entity expansion during XML parsing for security reasons.

However, many XSLTs define the following entity:
<!ENTITY nbsp "&#160;" >

They don't use the entity &nbsp, but its presence could lead to confusion.

Signed-off-by: Eden Bacani <eden.bacani@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2021-01-21 23:45:14 UTC
Created attachment 115652 [details] [review]
Bug 25381: (QA follow-up) Fix occurence in the one file that escaped

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Jonathan Druart 2021-01-22 13:54:00 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 12 Fridolin Somers 2021-01-29 13:18:21 UTC
Pushed to 20.11.x for 20.11.03
Comment 13 Andrew Fuerste-Henry 2021-02-03 22:07:02 UTC
Pushed to 20.05.x for 20.05.09
Comment 14 Victor Grousset/tuxayo 2021-02-03 23:10:50 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.