Bug 19613 - Scrub borrowers fields: borrowernotes opacnote
Summary: Scrub borrowers fields: borrowernotes opacnote
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 36328
Blocks:
  Show dependency treegraph
 
Reported: 2017-11-13 03:45 UTC by Amit Gupta
Modified: 2024-05-15 20:50 UTC (History)
17 users (show)

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


Attachments
Bug 19613 - Fix XSS in /cgi-bin/koha/circ/circulation.pl (1.16 KB, patch)
2017-11-13 03:48 UTC, Amit Gupta
Details | Diff | Splinter Review
Bug 19613 - Fix XSS in /cgi-bin/koha/circ/circulation.pl (1.22 KB, patch)
2017-11-13 05:51 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 19613: Scrub borrowers fields: borrowernotes opacnote (2.90 KB, patch)
2024-03-15 10:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19613: Use the 'note' profile (860 bytes, patch)
2024-03-20 07:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19613: Scrub borrowers fields: borrowernotes opacnote (2.95 KB, patch)
2024-03-21 01:01 UTC, David Cook
Details | Diff | Splinter Review
Bug 19613: Use the 'note' profile (913 bytes, patch)
2024-03-21 01:02 UTC, David Cook
Details | Diff | Splinter Review
Bug 19613: Scrub borrowers fields: borrowernotes opacnote (3.00 KB, patch)
2024-04-01 16:29 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 19613: Use the 'note' profile (1.14 KB, patch)
2024-04-01 16:29 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Amit Gupta 2017-11-13 03:45:22 UTC
Field: opacnote
Comment 1 Amit Gupta 2017-11-13 03:48:22 UTC
Created attachment 69104 [details] [review]
Bug 19613 - Fix XSS in /cgi-bin/koha/circ/circulation.pl

To Test
1. Hit the page /cgi-bin/koha/members/memberentry.pl
2. Add a text in the field opacnote that contains js
3. Save the page.
4. Notice js is execute
5. Apply patch and reload, the js is escaped
Comment 2 Chris Cormack 2017-11-13 05:51:23 UTC
Created attachment 69109 [details] [review]
Bug 19613 - Fix XSS in /cgi-bin/koha/circ/circulation.pl

To Test
1. Hit the page /cgi-bin/koha/members/memberentry.pl
2. Add a text in the field opacnote that contains js
3. Save the page.
4. Notice js is execute
5. Apply patch and reload, the js is escaped

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 3 Katrin Fischer 2017-12-03 13:22:55 UTC
Awaiting QA!
Comment 4 Katrin Fischer 2017-12-03 14:52:12 UTC
I think this change could be controversial as it will also disallow to use HTML in the note. Some libraries could see this as an unwanted change of behaviour. Could we create a filter that is more smart and only kills JavaScript?
Comment 5 Jonathan Druart 2024-03-15 10:48:07 UTC
Created attachment 163194 [details] [review]
Bug 19613: Scrub borrowers fields: borrowernotes opacnote

To prevent XSS
Comment 6 Jonathan Druart 2024-03-20 07:35:42 UTC
Created attachment 163498 [details] [review]
Bug 19613: Use the 'note' profile
Comment 7 Liz Rea 2024-03-20 13:35:58 UTC
Maybe instead, notes need a wysiwyg editor that only allows the most benign types of html
Comment 8 Owen Leonard 2024-03-20 14:05:55 UTC
Should the template be modified in this patch to use the $raw filter when showing opacnote? We do that for borrowernotes on moremember.pl.
Comment 9 David Cook 2024-03-21 01:00:14 UTC
I'm not sure about this one. 

It'll scrub new notes added, but if you already have bad notes they won't be scrubbed when you re-save the patron record. It also doesn't stop bad notes from being displayed. 

But I think it's a start, so I think I'll Sign Off.
Comment 10 David Cook 2024-03-21 01:01:57 UTC
Created attachment 163585 [details] [review]
Bug 19613: Scrub borrowers fields: borrowernotes opacnote

To prevent XSS

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 11 David Cook 2024-03-21 01:02:00 UTC
Created attachment 163586 [details] [review]
Bug 19613: Use the 'note' profile

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 12 Nick Clemens (kidclamp) 2024-04-01 16:29:10 UTC
Created attachment 164211 [details] [review]
Bug 19613: Scrub borrowers fields: borrowernotes opacnote

To prevent XSS

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Nick Clemens (kidclamp) 2024-04-01 16:29:13 UTC
Created attachment 164212 [details] [review]
Bug 19613: Use the 'note' profile

WNC amended patch: tidied

Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 wainuiwitikapark 2024-04-24 01:58:44 UTC
This doesn't apply cleanly to 22.05.x - could it please be rebased for the branch?
Comment 15 Frédéric Demians 2024-04-24 10:42:18 UTC
Applied to 22.11. Backporting 'note' C4::Scrubber profile. May be necessary for 22.05.
Comment 16 Tomás Cohen Arazi 2024-05-15 20:50:06 UTC
Pushed for 24.05!

Well done everyone, thank you!