Bug 39317 - Saving a letter template can lead to a CSRF error on some installs
Summary: Saving a letter template can lead to a CSRF error on some installs
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Baptiste Wojtkowski (bwoj)
QA Contact: David Cook
URL:
Keywords: rel_24_11_candidate
Depends on:
Blocks:
 
Reported: 2025-03-12 13:35 UTC by Baptiste Wojtkowski (bwoj)
Modified: 2025-06-10 11:45 UTC (History)
5 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.00
Circulation function:


Attachments
Bug 39317: Fix CSRF protection for letters (1.32 KB, patch)
2025-03-12 13:47 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 39317: Fix CSRF protection for letters (1.52 KB, patch)
2025-03-12 14:16 UTC, Baptiste Wojtkowski (bwoj)
Details | Diff | Splinter Review
Bug 39317: Fix CSRF protection for letters (1.72 KB, patch)
2025-04-11 08:43 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 39317: Fix CSRF protection for letters (1.77 KB, patch)
2025-04-14 00:47 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 Baptiste Wojtkowski (bwoj) 2025-03-12 13:35:27 UTC
STEPS TO REPRODUCE:
1 - go to /cgi-bin/koha/tools/letter.pl
2 - click on New notice -> claim serial issue (or any other one)
3 - Fill required fields with random string
4 - Save -> KABOOM
Comment 1 Baptiste Wojtkowski (bwoj) 2025-03-12 13:47:16 UTC Comment hidden (obsolete)
Comment 2 Baptiste Wojtkowski (bwoj) 2025-03-12 14:08:52 UTC
I'm a bit disappointed because the promised Kaboom doesn't appear on every install :'(
However there is an issue with the action submitted in post, which is by default the one which lead to the form, and which is incorrect. I'm updating the test plan
Comment 3 Baptiste Wojtkowski (bwoj) 2025-03-12 14:16:31 UTC Comment hidden (obsolete)
Comment 4 David Nind 2025-03-29 10:07:28 UTC
I tried several times, with different browsers, and couldn't get a KABOOM.

Hopefully someone else is more lucky!
Comment 5 Baptiste Wojtkowski (bwoj) 2025-03-31 07:51:22 UTC
(In reply to David Nind from comment #4)
> I tried several times, with different browsers, and couldn't get a KABOOM.
> 
> Hopefully someone else is more lucky!

I submitted a plan to test it without crash, see https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39317#c3
Comment 6 David Nind 2025-04-01 10:34:39 UTC
(In reply to Baptiste Wojtkowski (bwoj) from comment #5)
> (In reply to David Nind from comment #4)
> > I tried several times, with different browsers, and couldn't get a KABOOM.
> > 
> > Hopefully someone else is more lucky!
> 
> I submitted a plan to test it without crash, see
> https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39317#c3

Sorry, I still can't produce an error.
Comment 7 Magnus Enger 2025-04-01 14:47:14 UTC
No KABOOM for me either. What is the reasoning behind this patch? Is the action attribute required for the CSRF protection to work?
Comment 8 Phil Ringnalda 2025-04-10 21:56:08 UTC
Sounds like the missing Step 0 would be to install a reverse proxy in front of Koha which does something dubious when it sees a POST to a URL with a query string. Is it converting the POST to a GET, or stuffing the params in the query string into the POST body?
Comment 9 David Cook 2025-04-11 02:37:27 UTC
Does the patch solve your problem?

I don't love form elements without action attributes, so it seems like a reasonable change in general to me.
Comment 10 Baptiste Wojtkowski (bwoj) 2025-04-11 07:22:02 UTC
In my case ? Yup
Comment 11 Magnus Enger 2025-04-11 08:43:13 UTC
Created attachment 180818 [details] [review]
Bug 39317: Fix CSRF protection for letters

TEST PLAN:
1 - go to /cgi-bin/koha/tools/letter.pl
2 - click on New notice -> claim serial issue (or any other one)
3 - Fill required fields with random string
4 - Save -> if it breaks, go to
5 - Open a network panel (F12 -> network)
6 - Filter on letter.pl
7 - Fill another new notice -> the arguments of the form page have been
  passed in the address of the POST request.
8 - Apply patch
9 - Repeat the whole process and see it works now

Signed-off-by: Magnus Enger <magnus@libriotech.no>
I can't recreate the problem, but the change seems sensible enough
and does not seem to introduce a regression, I can still save a
new letter/notice.
Comment 12 David Cook 2025-04-13 23:46:24 UTC
I'll look at QAing this soon.

As Phil said, I wonder if this is something that Baptiste has noticed on BibLibre installs which run behind Nginx I believe?
Comment 13 David Cook 2025-04-14 00:47:50 UTC
Created attachment 180893 [details] [review]
Bug 39317: Fix CSRF protection for letters

TEST PLAN:
1 - go to /cgi-bin/koha/tools/letter.pl
2 - click on New notice -> claim serial issue (or any other one)
3 - Fill required fields with random string
4 - Save -> if it breaks, go to
5 - Open a network panel (F12 -> network)
6 - Filter on letter.pl
7 - Fill another new notice -> the arguments of the form page have been
  passed in the address of the POST request.
8 - Apply patch
9 - Repeat the whole process and see it works now

Signed-off-by: Magnus Enger <magnus@libriotech.no>
I can't recreate the problem, but the change seems sensible enough
and does not seem to introduce a regression, I can still save a
new letter/notice.
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 14 David Cook 2025-04-14 00:48:19 UTC
I couldn't reproduce the problem either, but it's a sensible change (ie improvement) to make and it works.
Comment 15 Katrin Fischer 2025-04-22 06:50:21 UTC
Pushed for 25.05!

Well done everyone, thank you!