Bug 24878 - Authentication check missing on calendar tools
Summary: Authentication check missing on calendar tools
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Authentication (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-16 15:21 UTC by Jonathan Druart
Modified: 2020-03-26 11:36 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.04, 19.05.09, 18.11.15


Attachments
Bug 24878: Add authentication checks to the calendar tool (1.37 KB, patch)
2020-03-16 15:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24878: Add authentication checks to the calendar tool (2.26 KB, patch)
2020-03-16 23:53 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 24878: Add authentication checks to the calendar tool (1.95 KB, patch)
2020-03-17 10:35 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24878: Add auth check for copy-holidays (773 bytes, patch)
2020-03-17 10:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24878: Add auth check for copy-holidays (829 bytes, patch)
2020-03-17 10:43 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-03-16 15:21:33 UTC
Which mean unauthorized users can edit (add/edit/delete) holidays.
Comment 1 Jonathan Druart 2020-03-16 15:28:22 UTC
Created attachment 100764 [details] [review]
Bug 24878: Add authentication checks to the calendar tool

There is a security hole in 2 scripts that are used by the UI to edit
holidays.
Comment 2 Jonathan Druart 2020-03-16 15:30:06 UTC
The lack of test plan is expected here, curl is your friend...
Comment 3 Katrin Fischer 2020-03-16 22:37:33 UTC
(In reply to Jonathan Druart from comment #2)
> The lack of test plan is expected here, curl is your friend...

Not sure I agree here...
Comment 4 Katrin Fischer 2020-03-16 22:40:13 UTC
(In reply to Katrin Fischer from comment #3)
> (In reply to Jonathan Druart from comment #2)
> > The lack of test plan is expected here, curl is your friend...
> 
> Not sure I agree here...

I would have tested this with a test plan, but it's really too late here to learn curl and how to apply it to this patch correctly.
Comment 5 Bernardo Gonzalez Kriegel 2020-03-16 23:53:50 UTC
Created attachment 100837 [details] [review]
Bug 24878: Add authentication checks to the calendar tool

There is a security hole in 2 scripts that are used by the UI to edit
holidays.

To test:
1) Go to Tools -> Calendar, for Centerville
   Check no holiday for 30/4/2020
2) To add a new holiday without login execute

   curl -d "newBranchName=CPL&showDayname=Mondays&newWeekday=1&newDay=30&newMonth=3&newYear=2020&dateofrange=27%2F03%2F2020&newTitle=Test+holiday&newDescription=Holy+description&newOperation=holiday&submit=Save" -H "Content-Type: application/x-www-form-urlencoded" -X POST http://staff/cgi-bin/koha/tools/newHolidays.pl

   Where 'staff' is your host:port
   This adds a new holiday for CPL (Centerville) on 30/4/2020
3) Reload page from 1), verify the new holiday
   edit and delete the holiday
4) Apply the patch
5) Do 2) again, this time you get a lengthy output,
   with the magic words:

   <title>Koha &rsaquo;
       Log in to Koha
   </title>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Only tested newHoliday but the fix is the same.
No errors
Comment 6 Jonathan Druart 2020-03-17 08:06:50 UTC
I personally do not think we should provide/publish a command that could be easily reused.
Comment 7 Katrin Fischer 2020-03-17 08:30:32 UTC
If you set the comment to private, only a select group will be able to see it even once the bug is published.
Comment 8 Nick Clemens 2020-03-17 10:35:02 UTC
Created attachment 100882 [details] [review]
Bug 24878: Add authentication checks to the calendar tool

There is a security hole in 2 scripts that are used by the UI to edit
holidays.

To test:
1) Go to Tools -> Calendar, for Centerville
   Check no holiday for 30/4/2020
2) To add a new holiday without login execute
   a curl command with necessary parameters
3) Reload page from 1), verify the new holiday
   edit and delete the holiday
4) Apply the patch
5) Do 2) again, this time you get a lengthy output,
   with the magic words:

   <title>Koha &rsaquo;
       Log in to Koha
   </title>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Only tested newHoliday but the fix is the same.
No errors

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 9 Jonathan Druart 2020-03-17 10:37:25 UTC
Created attachment 100883 [details] [review]
Bug 24878: Add auth check for copy-holidays
Comment 10 Nick Clemens 2020-03-17 10:43:43 UTC
Created attachment 100884 [details] [review]
Bug 24878: Add auth check for copy-holidays

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Bernardo Gonzalez Kriegel 2020-03-18 02:22:39 UTC
(In reply to Jonathan Druart from comment #6)
> I personally do not think we should provide/publish a command that could be
> easily reused.

You are right, sorry.
Joust wanted to show how it could be tested.
I found my way using tcpdump and wireshark.
Comment 12 Joy Nelson 2020-03-23 23:53:11 UTC
Pushed to 19.11.x branch for 19.11.04
Comment 13 Hayley Mapley 2020-03-24 03:18:44 UTC
Backported to 18.11.x for 18.11.15
Comment 14 Lucas Gass 2020-03-24 20:48:43 UTC
backported to 19.05.x for 19.05.09
Comment 15 Martin Renvoize 2020-03-26 11:36:36 UTC
Pushed to master