Bug 28947 - OPAC user can create new users
Summary: OPAC user can create new users
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 28929
Blocks:
  Show dependency treegraph
 
Reported: 2021-09-03 09:55 UTC by Jonathan Druart
Modified: 2022-06-06 20:25 UTC (History)
22 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.04, 20.11.10, 20.05.16, 19.11.22


Attachments
Bug 28947: Prevent OPAC user to create new users (862 bytes, patch)
2021-09-03 10:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28947: Prevent OPAC user to create new users (4.10 KB, patch)
2021-09-03 10:41 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28947: Prevent OPAC user to create new users (1.22 KB, patch)
2021-09-06 07:04 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 Jonathan Druart 2021-09-03 09:55:39 UTC
Another bad one

http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create

Fill the form, submit, cry
Comment 1 Marcel de Rooy 2021-09-03 10:00:37 UTC
This sounds very bad too.
Comment 2 Jonathan Druart 2021-09-03 10:01:59 UTC
Created attachment 124451 [details] [review]
Bug 28947: Prevent OPAC user to create new users
Comment 3 Jonathan Druart 2021-09-03 10:02:43 UTC
BibLibre team found this!
Comment 4 Jonathan Druart 2021-09-03 10:03:34 UTC
Test self-reg(with and without PatronSelfRegistration+ with and without PatronSelfRegistrationVerifyByEmail) + self-mod.
Comment 5 Nick Clemens 2021-09-03 10:41:03 UTC
Created attachment 124452 [details] [review]
Bug 28947: Prevent OPAC user to create new users

This patch prevents an existing user from exploiting the patron edit form in order to
force create new patrons

To test:
1 - Test with PatronSelfRegistration:Don't Allow PatronSelfRegistrationVerifyByEmail:Don't Require
    - sign in to opac
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - Fill out form - patron is created FAIL
    - sign out
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - you are sent to opac-main
2 - Test with PatronSelfRegistration:Don't Allow PatronSelfRegistrationVerifyByEmail:Require
    - sign in to opac
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - Fill out form - patron is sent email/will be created FAIL
    - sign out
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - you are sent to opac-main
3 - Test with PatronSelfRegistration:Allow PatronSelfRegistrationVerifyByEmail:Don't Require
    - sign in to opac
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - Fill out form - patron is created FAIL
    - sign out
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - patron is created OK
4 - Test with PatronSelfRegistration:Allow PatronSelfRegistrationVerifyByEmail:Require
    - sign in to opac
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - Fill out form - patron is sent email/will be created FAIL
    - sign out
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - patron is sent email/will be created OK
5 - APPLY PATCH
6 - Test with PatronSelfRegistration:Don't Allow PatronSelfRegistrationVerifyByEmail:Don't Require
    - sign in to opac
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - You are sent to opac-main
    - sign out
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - You are sent to opac-main
7 - Test with PatronSelfRegistration:Don't Allow PatronSelfRegistrationVerifyByEmail:Require
    - sign in to opac
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - You are sent to opac-main
    - sign out
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - You are sent to opac-main
8 - Test with PatronSelfRegistration:Allow PatronSelfRegistrationVerifyByEmail:Don't Require
    - sign in to opac
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - You are sent to opac-main
    - sign out
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - patron is created OK
9 - Test with PatronSelfRegistration:Allow PatronSelfRegistrationVerifyByEmail:Require
    - sign in to opac
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - You are sent to opac-main
    - sign out
    - go to http://kohadev.mydnsname.org:8080/cgi-bin/koha/opac-memberentry.pl?action=create
    - patron is sent email/will be created OK

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 6 Katrin Fischer 2021-09-03 11:32:46 UTC
Should the exploitation instructions be in the commit message in this case? Just unsure about releasing instructions possibly before people had a fair chance to patch.
Comment 7 Nick Clemens 2021-09-03 11:54:42 UTC
(In reply to Katrin Fischer from comment #6)
> Should the exploitation instructions be in the commit message in this case?
> Just unsure about releasing instructions possibly before people had a fair
> chance to patch.

Fair point, I just wanted to get them listed. Please remove on QA :-)
Comment 8 Marcel de Rooy 2021-09-06 06:57:48 UTC
QA: Looking here now
Comment 9 Marcel de Rooy 2021-09-06 07:04:56 UTC
Created attachment 124533 [details] [review]
Bug 28947: Prevent OPAC user to create new users

This patch prevents an existing user from exploiting the patron edit form in order to
force create new patrons

To test:
Try all combinations of PatronSelfRegistration and PatronSelfRegistrationVerifyByEmail
with and without this patch.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2021-09-06 07:06:46 UTC
There is confusion about the $action parameter.
Someone may have introduced new at some point while it should have been create which is used more widely in the script/template.
The script and template contain only a few references to new. Probably better to replace them (on another report)?
Comment 11 Katrin Fischer 2021-09-06 07:39:00 UTC
Is it just the naming or something wrong with the patch itself you are worried about?
Comment 12 Marcel de Rooy 2021-09-06 07:46:31 UTC
(In reply to Katrin Fischer from comment #11)
> Is it just the naming or something wrong with the patch itself you are
> worried about?

No, this inconsistency is already before the patch. But the patch adds another instance of action=new. Can be fixed elsewhere.
Comment 13 Katrin Fischer 2021-09-06 08:06:22 UTC
Verified the problem on our 18.11.16 installations and can confirm that the patch has the intended effect. Also tested that with the patch applied address changes and patron self registration when acticated still work as expected.
Comment 14 Victor Grousset/tuxayo 2021-09-16 07:50:42 UTC
Backported: Pushed to 20.05.x security branch for 20.05.16

Testing was done with
- when logged in the opac
- going to http://localhost:8080/cgi-bin/koha/opac-memberentry.pl?action=create
- creating a user
- using F5 to create another one (to check the the server will actually reject the network request, not just forbid displaying the form)
- applying the patch
- using F5 and seeing that a 3rd user isn't created
Comment 15 Jonathan Druart 2021-09-30 07:22:45 UTC
Backported to 21.05.04, 20.11.10, 20.05.16, 19.11.22
Comment 16 Jonathan Druart 2021-09-30 07:52:18 UTC
Pushed to master for 21.11, thanks to everybody involved!