Bug 28947

Summary: OPAC user can create new users
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: critical    
Priority: P5 - low CC: 1joynelson, bastien.gonckel, corinne.hayet, didier.gautheron, f.demians, fridolin.somers, jonathan.druart, katrin.fischer, laurent.ducos, lucas, m.de.rooy, magnus, martin.renvoize, mengu, mik, nick, nicolas.legrand, roger.grossmann, sonia.bouis, ulrich.kleiber, victor, ztajoli
Version: unspecified   
Hardware: All   
OS: All   
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
Bug Depends on: 28929    
Bug Blocks:    
Attachments: Bug 28947: Prevent OPAC user to create new users
Bug 28947: Prevent OPAC user to create new users
Bug 28947: Prevent OPAC user to create new users

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!