Bug 31165 - "Public note" field in course reserve should restrict HTML usage
Summary: "Public note" field in course reserve should restrict HTML usage
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Course reserves (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: David Cook
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 38488
Blocks:
  Show dependency treegraph
 
Reported: 2022-07-15 15:49 UTC by Jimmy Bell
Modified: 2025-06-23 04:08 UTC (History)
15 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.01,25.05.00,24.11.03,24.05.08,23.11.13,22.11.25
Circulation function:


Attachments
4 images, shows where payload in entered and the resulting popup when opening the course reserve or listing it (195.15 KB, application/x-zip-compressed)
2022-07-15 15:49 UTC, Jimmy Bell
Details
Bug 31165: Scrub "Public note" field for Course Reserves (7.41 KB, patch)
2024-11-20 01:10 UTC, David Cook
Details | Diff | Splinter Review
Bug 31165: Scrub "Public note" field for Course Reserves (7.46 KB, patch)
2025-01-10 18:49 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 31165: Scrub "Public note" field for Course Reserves (6.99 KB, patch)
2025-02-15 05:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 31165: Scrub "Public note" field for Course Reserves (7.08 KB, patch)
2025-02-21 07:25 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[24.05.x] Bug 31165: Scrub "Public note" field for Course Reserves (7.93 KB, patch)
2025-03-24 05:03 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jimmy Bell 2022-07-15 15:49:52 UTC
Created attachment 137774 [details]
4 images, shows where payload in entered and the resulting popup when opening the course reserve or listing it

The vulnerability is present on the intranet of Koha (staff-side).
When adding a new course reserve, the staff member has the possibility to enter a "private/staff note" field and a "public note" field.

The public note field is vulnerable to XSS.

Entering <script>alert(1)</script> in this field (as written), then saving the new course reserve will cause the database to save the HTML and will trigger a popup alert at any time a user opens the course reserve. A user on the client side, such as a student, would also get the popup alert if he were to open the course reserve page.

The same problem has been found in the "public note" field when adding a copy of a book to a course, for example. The staff member once again has access to this field and can inject javascript there that is then triggered for everyone viewing it.

The severity appears minor since you have to have a staff account with adequate permissions to write in this "public note" field. The XSS likely could not be used to steal a session cookie considering that the cookie is set to HTTPOnly. 

Note that the vulnerability could be present on other "public note" fields in Koha that I couldn't validate.

I strongly advise filing for a CVE for this vulnerability that I have been able to exploit on Koha 21.11.06 and Koha 21.05.11, suggesting this might be present in many versions of the software.
Comment 1 Jonathan Druart 2022-07-25 07:38:21 UTC
This is the expected behaviour:
  commit d8187991a40f58ab815fd1e75d6a3671ca72a668
  Bug 23683: Course reserves public notes on specific items should allow for HTML
Comment 2 Jonathan Druart 2022-07-25 07:40:08 UTC
However we could use C4::Scrubber to reduce the HTML nodes allowed, but it's not allowing "a".
Comment 3 Jimmy Bell 2022-07-25 15:26:59 UTC
From a security perspective, it would indeed be ideal to disallow the script node and its variations, like ScriPT or any lowercase/uppercase combination.
Comment 4 David Cook 2022-07-25 23:49:55 UTC
(In reply to Jonathan Druart from comment #2)
> However we could use C4::Scrubber to reduce the HTML nodes allowed, but it's
> not allowing "a".

That's an interesting idea... 

(In reply to Jimmy Bell from comment #3)
> From a security perspective, it would indeed be ideal to disallow the script
> node and its variations, like ScriPT or any lowercase/uppercase combination.

Maybe although there's a million other ways of doing XSS beyond just the "script" tag. A very common other one is the "img" tag, but really you can use just about anything. It's a bit of a losing battle when you try to exclude some HTML instead of all HTML. 

That said, I suppose you could argue it's similar to locking the door to your house or car. It's not going to stop a determined thief, who might just break a window to gain entry, but it'll still deter the majority of thieves.

Since the issue exists in the staff interface with trusted users, I don't know that I'd class this as a vulnerability (like I would for an unauthenticated public user), but scrubbing some HTML couldn't hurt. 

One could argue that while "OPACUserJS" gives an admin the ability to write arbitrary Javascript, perhaps someone with Course Reserve access isn't trusted to that same level as the admin, and so we need to add some further restrictions. 

Although if we start to pull that thread... it could get very complicated. 

Anyway, interesting ideas!
Comment 5 Jimmy Bell 2022-07-26 19:11:03 UTC
The way I see it is that the fact that the staff interface allows only for trusted users does not make it less of a vulnerability, it just decreases the likelihood that it could be exploited.

On the Koha instance I tested this on, the password policy for admin and staff on the intranet was very weak, something like a minimum of 3 characters. That would be easy to bruteforce if the username is known, and often in library settings, credentials are put on a post-it near the staff computer.

I agree that all this is a complicated scenario and that this XSS is not very likely to be exploited. Deciding to patch it or not and to what extent is up to you, but in my opinion it still qualifies as a vulnerability. Session cookies are unlikely to be stolen because of their httponly and secure flags, however through XSS, users could be redirected to a malicious website and a malicious employee could trick them in giving away any kind of passwords.

Whether or not you choose to patch this, having a CVE with a high privileges required vector is still advised since it can provide the community with the knowledge that it exists.

Thanks :)
Comment 6 David Cook 2022-07-27 02:15:18 UTC
(In reply to Jimmy Bell from comment #5)
> The way I see it is that the fact that the staff interface allows only for
> trusted users does not make it less of a vulnerability, it just decreases
> the likelihood that it could be exploited.

Academically maybe but not in real world terms. (E.g. the only secure computer is one with no network connection and no users.)

Would you say that Drupal and Wordpress are vulnerable because they allow trusted users to add HTML, CSS, and Javascript? CMSes are designed to allow trusted users to add this code, and Koha has many CMS functions (e.g. "News", "HTML Customizations", etc.). 

In fact, I don't think you could even class this as a XSS vulnerability, because XSS implies that the attacker is external to the site and they've injected the script via a URL or stored user input (like a text comment). In your scenario, the attacker already controls the system. It's just a compromised website.

> On the Koha instance I tested this on, the password policy for admin and
> staff on the intranet was very weak, something like a minimum of 3
> characters. That would be easy to bruteforce if the username is known, and
> often in library settings, credentials are put on a post-it near the staff
> computer.

Out of the box, Koha does make some unfortunate choices regarding password policy and account security. 

The system preference "FailedLoginAttempts" should be set out of the box, and the default password policy should be much higher and more complex. There are existing tickets for password length but they always seems to stall. 

And yeah people have some pretty poor password management behaviours.

Of course, even when people have strong password management and 2FA, a determined attacker can still break into systems. The latest hack of Microsoft apparently involved someone triggering the 2FA challenge so many times that the personal responsible just hit "Approve" to stop the alerts from coming to their authenticator app. Humans are often the weakest link.

> I agree that all this is a complicated scenario and that this XSS is not
> very likely to be exploited. Deciding to patch it or not and to what extent
> is up to you, but in my opinion it still qualifies as a vulnerability.
> Session cookies are unlikely to be stolen because of their httponly and
> secure flags, however through XSS, users could be redirected to a malicious
> website and a malicious employee could trick them in giving away any kind of
> passwords.

Well, it's not up to me ;). You're free to submit a patch and leave it up to people to test and QA. I'm not a gatekeeper.

> Whether or not you choose to patch this, having a CVE with a high privileges
> required vector is still advised since it can provide the community with the
> knowledge that it exists.

Feel free to create a CVE. As I said above, many features in Koha rely on authenticated authorized staff users inserting HTML, Javascript, and CSS. You'd probably want to include all those features in your CVE.

That all said, perhaps Koha should have a file-based configuration option on the server that controls whether or not web users can enter HTML, CSS, and Javascript which is disables those features out of the box. Feel free to send a patch with the code to achieve that.
Comment 7 David Cook 2022-07-27 02:22:58 UTC
I think I'm done commenting on this one. 

Jonathan's scrubbing idea is interesting. It would be worthwhile checking with the folk from Bug 23683 to see what HTML elements they need, and then only allowing those through. It's not a bad idea.

But this isn't a stored XSS vulnerability, so the bug report title should be changed to something like: '"Public note" field in Course Reserve should restrict HTML usage'

A stored XSS vulnerability would be if an OPAC user could add a comment with Javascript to the Course Reserve. 

The vulnerability in the scenario about someone taking over a staff account to inject Javascript into a Course Reserve note would be with the authentication process - not the CMS feature.
Comment 8 Jimmy Bell 2022-07-27 14:11:24 UTC
I see your point.

I'm by no means a Koha expert, this was found during an authorized pentesting exercise where I work. If this constitutes a feature, I don't believe a CVE would be appropriate anymore.

Restricting the HTML is indeed a great idea if someone ever wants to look into that.

Thank you all for taking the time, hope this can help in some way or another.

I will now edit the title of the Bug as David suggested.
Comment 9 David Cook 2024-11-20 00:38:55 UTC
My stance on this one has softened or perhaps hardened over the past few years, and I'm more inclined to scrub user-entered HTML wherever possible, even when entered by trusted staff members.

I'm going to provide a patch using a new technique. We might find we don't want to use that technique, and instead use an older technique, but going to move forward on this one anyway...
Comment 10 David Cook 2024-11-20 01:10:08 UTC
Created attachment 174816 [details] [review]
Bug 31165: Scrub "Public note" field for Course Reserves

When displaying the "Public note" for Course Reserves, we scrub the HTML
using the "note" scrubber type.

Test plan:
0. Apply the patch
1. Go to http://localhost:8081/cgi-bin/koha/course_reserves/course-reserves.pl
2. Click "New course"
3. Fill in required fields
4. In staff note and public note add the following:
<script>alert('boo!')</script><p>Hello!</p>
5. Click Save

6. Click "Add reserves"
7. Add "39999000001310" to Item barcode
8. Click "Submit"
9. Fill in all required fields
10. Add the following for Staff note and Public note:
<script>alert('boo2!')</script><p>Item!</p>
11. Click "Save"

12. Go back to your course reserve
13. Note the HTML is scrubbed on "Public note"
14. Go to http://localhost:8081/cgi-bin/koha/course_reserves/course-reserves.pl
15. Note the HTML is scrubbed on "Public note"

16. Go to http://localhost:8080/cgi-bin/koha/opac-course-reserves.pl
17. Note the HTML is scrubbed on "Notes"
18. Click on your course reserves
19. Note the HTML is scrubbed on "Notes"
Comment 11 David Cook 2024-11-20 01:12:46 UTC
I think someone could make the argument that we should be handling the scrubbing in the .pl/.pm code, but it can involve double-handling data and you have to make sure you're just scrubbing at display time and not edit time (unless you don't mind silently erasing data).

Since we use the $raw filter in the templates, I think using bug 38488 as the basis for this actually makes a lot of sense. 

Happy to chat about this more with QA though...
Comment 12 Brendan Lawlor 2025-01-10 18:49:23 UTC
Created attachment 176386 [details] [review]
Bug 31165: Scrub "Public note" field for Course Reserves

When displaying the "Public note" for Course Reserves, we scrub the HTML
using the "note" scrubber type.

Test plan:
0. Apply the patch
1. Go to http://localhost:8081/cgi-bin/koha/course_reserves/course-reserves.pl
2. Click "New course"
3. Fill in required fields
4. In staff note and public note add the following:
<script>alert('boo!')</script><p>Hello!</p>
5. Click Save

6. Click "Add reserves"
7. Add "39999000001310" to Item barcode
8. Click "Submit"
9. Fill in all required fields
10. Add the following for Staff note and Public note:
<script>alert('boo2!')</script><p>Item!</p>
11. Click "Save"

12. Go back to your course reserve
13. Note the HTML is scrubbed on "Public note"
14. Go to http://localhost:8081/cgi-bin/koha/course_reserves/course-reserves.pl
15. Note the HTML is scrubbed on "Public note"

16. Go to http://localhost:8080/cgi-bin/koha/opac-course-reserves.pl
17. Note the HTML is scrubbed on "Notes"
18. Click on your course reserves
19. Note the HTML is scrubbed on "Notes"

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 13 Brendan Lawlor 2025-01-10 18:54:40 UTC
Thanks for the excellent test plan.

This was a cool example to see HtmlScrubber in action!
Comment 14 David Cook 2025-01-12 23:05:31 UTC
(In reply to Brendan Lawlor from comment #13)
> Thanks for the excellent test plan.
> 
> This was a cool example to see HtmlScrubber in action!

Thanks, Brendan!

I'd just add a note that the dependency bug 38488 needs a sign off too! Thanks!
Comment 15 Julian Maurice 2025-01-30 12:41:14 UTC
(In reply to Jonathan Druart from comment #2)
> However we could use C4::Scrubber to reduce the HTML nodes allowed, but it's
> not allowing "a".
If I read the patches correctly, nothing is done to allow <a>. Won't it be a problem ? I imagine it can be useful in a public note.
Comment 16 Marcel de Rooy 2025-02-14 08:00:21 UTC
Have some fun with auto_rebase ;) Sorry, but we're all in the same boat now..
Comment 17 Jonathan Druart 2025-02-15 05:31:34 UTC
Created attachment 178125 [details] [review]
Bug 31165: Scrub "Public note" field for Course Reserves

When displaying the "Public note" for Course Reserves, we scrub the HTML
using the "note" scrubber type.

Test plan:
0. Apply the patch
1. Go to http://localhost:8081/cgi-bin/koha/course_reserves/course-reserves.pl
2. Click "New course"
3. Fill in required fields
4. In staff note and public note add the following:
<script>alert('boo!')</script><p>Hello!</p>
5. Click Save

6. Click "Add reserves"
7. Add "39999000001310" to Item barcode
8. Click "Submit"
9. Fill in all required fields
10. Add the following for Staff note and Public note:
<script>alert('boo2!')</script><p>Item!</p>
11. Click "Save"

12. Go back to your course reserve
13. Note the HTML is scrubbed on "Public note"
14. Go to http://localhost:8081/cgi-bin/koha/course_reserves/course-reserves.pl
15. Note the HTML is scrubbed on "Public note"

16. Go to http://localhost:8080/cgi-bin/koha/opac-course-reserves.pl
17. Note the HTML is scrubbed on "Notes"
18. Click on your course reserves
19. Note the HTML is scrubbed on "Notes"

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 18 Jonathan Druart 2025-02-15 05:31:51 UTC
(In reply to Marcel de Rooy from comment #16)
> Have some fun with auto_rebase ;) Sorry, but we're all in the same boat now..

$ perl auto_rebase.pl 
Creating a temporary branch 'bug_31165-tmp'...
Rebasing the temporary branch up to before the tidy commits...
Creating a new branch 'bug_31165-rebased' starting at the end of the tidy commits...
Getting commit list from branch 'bug_31165-tmp'...
No number of commits passed, trying to guess it...
Found 1 commits to process
Processing 1 commits...
        Processing commit 62ca8ede53a... (1/1)
                Processing file koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt... (1/4)
Tidying file 1/1 (koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt)
                Processing file koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-reserves.tt... (2/4)
Tidying file 1/1 (koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-reserves.tt)
                Processing file koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-course-details.tt... (3/4)
Tidying file 1/1 (koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-course-details.tt)
                Processing file koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-course-reserves.tt... (4/4)
Tidying file 1/1 (koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-course-reserves.tt)
                Committing changes...

All commits from the source branch have been applied to 'bug_31165-rebased'!


Rebasing on top of main...
Everything applied successfully!
Comment 19 David Cook 2025-02-16 22:20:30 UTC
(In reply to Julian Maurice from comment #15)
> (In reply to Jonathan Druart from comment #2)
> > However we could use C4::Scrubber to reduce the HTML nodes allowed, but it's
> > not allowing "a".
> If I read the patches correctly, nothing is done to allow <a>. Won't it be a
> problem ? I imagine it can be useful in a public note.

Honestly, none of my libraries use Course Reserves, so I don't have production data to work with here. Happy for people to add more scrubber profiles. Or take a look at bug 31934 for a more customizable approach.
Comment 20 Marcel de Rooy 2025-02-21 07:23:55 UTC
[% course.public_note | scrub_html type => 'note' | $raw %]

We dont need $raw here anymore. But no problem.
Comment 21 Marcel de Rooy 2025-02-21 07:25:19 UTC
Created attachment 178443 [details] [review]
Bug 31165: Scrub "Public note" field for Course Reserves

When displaying the "Public note" for Course Reserves, we scrub the HTML
using the "note" scrubber type.

Test plan:
0. Apply the patch
1. Go to http://localhost:8081/cgi-bin/koha/course_reserves/course-reserves.pl
2. Click "New course"
3. Fill in required fields
4. In staff note and public note add the following:
<script>alert('boo!')</script><p>Hello!</p>
5. Click Save

6. Click "Add reserves"
7. Add "39999000001310" to Item barcode
8. Click "Submit"
9. Fill in all required fields
10. Add the following for Staff note and Public note:
<script>alert('boo2!')</script><p>Item!</p>
11. Click "Save"

12. Go back to your course reserve
13. Note the HTML is scrubbed on "Public note"
14. Go to http://localhost:8081/cgi-bin/koha/course_reserves/course-reserves.pl
15. Note the HTML is scrubbed on "Public note"

16. Go to http://localhost:8080/cgi-bin/koha/opac-course-reserves.pl
17. Note the HTML is scrubbed on "Notes"
18. Click on your course reserves
19. Note the HTML is scrubbed on "Notes"

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 22 Alex Buckley 2025-03-20 23:42:34 UTC
Hi, 

This conflicts when applied on 24.05.x-security.

---

kohadev-koha@kohadevbox:koha((v24.05.07))$ git bz apply 31165

Bug 31165 - "Public note" field in course reserve should restrict HTML usage

178443 - Bug 31165: Scrub "Public note" field for Course Reserves

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 31165: Scrub "Public note" field for Course Reserves
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-reserves.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-course-details.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-course-reserves.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-course-reserves.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-course-reserves.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-course-details.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-course-details.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-reserves.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-reserves.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 31165: Scrub "Public note" field for Course Reserves
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-31165-Scrub-Public-note-field-for-Course-Reser-ipqwckct.patch
kohadev-koha@kohadevbox:koha((v24.05.07)|AM 1/1)$ 

---

Could we please have a rebase?
Comment 23 David Cook 2025-03-24 05:03:01 UTC
Created attachment 179635 [details] [review]
[24.05.x] Bug 31165: Scrub "Public note" field for Course Reserves

When displaying the "Public note" for Course Reserves, we scrub the HTML
using the "note" scrubber type.

Test plan:
0. Apply the patch
1. Go to http://localhost:8081/cgi-bin/koha/course_reserves/course-reserves.pl
2. Click "New course"
3. Fill in required fields
4. In staff note and public note add the following:
<script>alert('boo!')</script><p>Hello!</p>
5. Click Save

6. Click "Add reserves"
7. Add "39999000001310" to Item barcode
8. Click "Submit"
9. Fill in all required fields
10. Add the following for Staff note and Public note:
<script>alert('boo2!')</script><p>Item!</p>
11. Click "Save"

12. Go back to your course reserve
13. Note the HTML is scrubbed on "Public note"
14. Go to http://localhost:8081/cgi-bin/koha/course_reserves/course-reserves.pl
15. Note the HTML is scrubbed on "Public note"

16. Go to http://localhost:8080/cgi-bin/koha/opac-course-reserves.pl
17. Note the HTML is scrubbed on "Notes"
18. Click on your course reserves
19. Note the HTML is scrubbed on "Notes"

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 24 Alex Buckley 2025-03-24 07:40:40 UTC
Thank you for the rebase. Tested, works as described.

Applied to 24.05.x-security
Comment 25 Fridolin Somers 2025-03-24 10:11:44 UTC
Applied to 23.11.x-security
Comment 26 Katrin Fischer 2025-04-14 06:52:05 UTC
Pushed for 25.05!

Well done everyone, thank you!