Bug 24878

Summary: Authentication check missing on calendar tools
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: AuthenticationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact:
Severity: critical    
Priority: P5 - low CC: 1joynelson, bgkriegel, chris, dpavlin, fridolin.somers, hayleypelham, jonathan.druart, katrin.fischer, lucas, martin.renvoize, nick
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24879
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
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
Bug 24878: Add authentication checks to the calendar tool
Bug 24878: Add authentication checks to the calendar tool
Bug 24878: Add auth check for copy-holidays
Bug 24878: Add auth check for copy-holidays

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 Pelham 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