Bug 19613 - Scrub borrowers fields: borrowernotes opacnote
Summary: Scrub borrowers fields: borrowernotes opacnote
Status: Pushed to oldoldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 36328
Blocks:
  Show dependency tree / graph
 
Reported: 2017-11-13 03:45 UTC by Amit Gupta
Modified: 2024-05-22 13:05 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:
24.05.00,23.11.05,23.05.11,22.11.17,22.05.21
Circulation function:

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
Show Obsolete (6) View All Add an attachment (proposed patch, testcase, etc.)

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 Wainui Witika-Park 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 (tcohen) 2024-05-15 20:50:06 UTC 
Pushed for 24.05!

Well done everyone, thank you!
Comment 17 Fridolin Somers 2024-05-22 13:05:14 UTC 
(In reply to wainuiwitikapark from comment #14)
> This doesn't apply cleanly to 22.05.x - could it please be rebased for the
> branch?

I see it in in 22.05.x