Bug 34369 - Add CSRF protection to system preferences
Summary: Add CSRF protection to system preferences
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: David Cook
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 30524
Blocks:
  Show dependency treegraph
 
Reported: 2023-07-25 05:23 UTC by David Cook
Modified: 2025-06-02 20:45 UTC (History)
11 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.04,22.11.10,22.05.16,21.11.24
Circulation function:


Attachments
Bug 34369: Require CSRF token for updating system preferences (7.77 KB, patch)
2023-08-01 06:58 UTC, David Cook
Details | Diff | Splinter Review
Bug 34369: Pass csrf token from syspref API client (for Vue app) (2.20 KB, patch)
2023-08-01 07:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34369: Require CSRF token for updating system preferences (7.85 KB, patch)
2023-08-03 08:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34369: Pass csrf token from syspref API client (for Vue app) (1.38 KB, patch)
2023-08-03 08:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34369: Fix 'Did you mean' (1.62 KB, patch)
2023-08-03 08:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34369: Require CSRF token for updating system preferences (7.93 KB, patch)
2023-08-04 07:03 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34369: Pass csrf token from syspref API client (for Vue app) (1.47 KB, patch)
2023-08-04 07:03 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 34369: Fix 'Did you mean' (1.72 KB, patch)
2023-08-04 07:03 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2023-07-25 05:23:20 UTC
The system preferences need urgent CSRF protection to prevent external XSS injection and other undesirable consequences.
Comment 1 David Cook 2023-08-01 03:55:35 UTC
This one is a bit harder to exploit but I've done it previously...
Comment 4 David Cook 2023-08-01 05:53:39 UTC
It looks like preferenes.pl $op save doesn't get used anymore as an old school API is used instead and systempreferences.pl is used for "Local use preferences"...

So we'll need to write a token out to preferences.pl, find it and send it in koha-tmpl/intranet-tmpl/prog/js/pages/preferences.js, and choke without it in ./svc/config/systempreferences
Comment 6 David Cook 2023-08-01 06:43:41 UTC
Bug 33289 is going to be a problem... since it doesn't have ready access to a CSRF token...
Comment 7 David Cook 2023-08-01 06:52:15 UTC
(In reply to David Cook from comment #6)
> Bug 33289 is going to be a problem... since it doesn't have ready access to
> a CSRF token...

I actually bumped into this same problem with a Vue app I created on Wordpress that uses the Wordpress API: https://developer.wordpress.org/rest-api/using-the-rest-api/authentication/

I think the official way and the way I ended up doing was to use the server-side to print out a data structure containing the CSRF token in an inline Javascript element.

Of course, that's going to be easier said than done in Koha.

I imagine Jonathan will have more insight into handle CSRF for Bug 33289...
Comment 8 David Cook 2023-08-01 06:58:06 UTC
Created attachment 154121 [details] [review]
Bug 34369: Require CSRF token for updating system preferences

This patch adds the requirements that updating a system preference
requires a CSRF token. (Also, adding and deleting local system preferences.)

0. Apply patch
1. koha-plack --reload kohadev
2. Add local system preference
3. Update local system preference
4. Delete local system preference
5. Update normal system preference
6. Note no errors
Comment 9 Jonathan Druart 2023-08-01 07:39:39 UTC
Created attachment 154122 [details] [review]
Bug 34369: Pass csrf token from syspref API client (for Vue app)
Comment 10 Jonathan Druart 2023-08-01 07:41:10 UTC
(In reply to Jonathan Druart from comment #9)
> Created attachment 154122 [details] [review] [review]
> Bug 34369: Pass csrf token from syspref API client (for Vue app)

This is not used for now (removed from bug 33606) but will be by bug 30708 (set sysprefs from the Vue app).
Comment 11 David Cook 2023-08-02 01:05:53 UTC
Thanks, Jonathan. I really appreciate that.
Comment 12 Jonathan Druart 2023-08-03 08:04:00 UTC
Created attachment 154163 [details] [review]
Bug 34369: Require CSRF token for updating system preferences

This patch adds the requirements that updating a system preference
requires a CSRF token. (Also, adding and deleting local system preferences.)

0. Apply patch
1. koha-plack --reload kohadev
2. Add local system preference
3. Update local system preference
4. Delete local system preference
5. Update normal system preference
6. Note no errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2023-08-03 08:04:03 UTC
Created attachment 154164 [details] [review]
Bug 34369: Pass csrf token from syspref API client (for Vue app)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Jonathan Druart 2023-08-03 08:04:07 UTC
Created attachment 154165 [details] [review]
Bug 34369: Fix 'Did you mean'

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Marcel de Rooy 2023-08-04 06:33:22 UTC
Looking here now
Comment 16 Marcel de Rooy 2023-08-04 07:03:39 UTC
Created attachment 154237 [details] [review]
Bug 34369: Require CSRF token for updating system preferences

This patch adds the requirements that updating a system preference
requires a CSRF token. (Also, adding and deleting local system preferences.)

0. Apply patch
1. koha-plack --reload kohadev
2. Add local system preference
3. Update local system preference
4. Delete local system preference
5. Update normal system preference
6. Note no errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 17 Marcel de Rooy 2023-08-04 07:03:42 UTC
Created attachment 154238 [details] [review]
Bug 34369: Pass csrf token from syspref API client (for Vue app)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 18 Marcel de Rooy 2023-08-04 07:03:44 UTC
Created attachment 154239 [details] [review]
Bug 34369: Fix 'Did you mean'

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 19 David Cook 2023-08-30 00:50:17 UTC
In terms of CSRF fixes, this one should be a priority I think, since it has a large blast radius.
Comment 20 Tomás Cohen Arazi (tcohen) 2023-09-25 22:08:33 UTC
Note: I had to rebase it. If the dependencies are not backported, you might need to pick the patches from this report.

Pushed to master for 23.11.

Nice work everyone, thanks!