The _add_default_csrf_params internal function accesses C4::Context->userenv without checking that it has been defined. I think not all of the potential callers of it declare that they require a defined userenv, so we should test and provide defaults for required values if it is not defined, to avoid some "Can't use an undefined value as a HASH reference" HTTP 500 Internal Server Errors. Step to Reproduce: write some code that results in that function being used before a set_userenv call Actual Result: error logged, code exits Expected Result: code runs to completion Additional Information: I am not sure whether this is currently triggered by any released core koha code, or only a few plugins and mods. It looks to me like a bug waiting to happen, based on the documentation, which can be avoided simply.
Created attachment 117744 [details] [review] [PATCH] Bug 27849: Koha::Token may access undefined C4::Context->userenv
Saluton :) Ĉu estas bona maniero por testi? Is that a good way to test: Add the following code: Koha::Token->new->check_csrf({ session_id => scalar $query->cookie('CGISESSID'), token => scalar $query->param('csrf_token'), }); In opac/opac-user.pl After C4::Context->preference('casAuthentication') and C4::Auth_with_cas::logout_if_required($query); And before my ( $template, $borrowernumber, $cookie ) = get_template_and_user( It seems to be the case since it crashes without the patch and work with it.
Created attachment 137802 [details] [review] Bug 27849: Koha::Token may access undefined C4::Context->userenv The _add_default_csrf_params internal function accesses C4::Context->userenv without checking that it has been defined. I think not all of the potential callers of it declare that they require a defined userenv, so we should test and provide defaults for required values if it is not defined, to avoid some "Can't use an undefined value as a HASH reference" HTTP 500 Internal Server Errors. To test: Do anything that requires a form with CSRF token, such as editing your details. Behaviour should be unchanged. To test the failure case, you would need some customised code that indirectly generates a CSRF token before setting the userenv up and I am not sure there is any in released Koha yet. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Looks good to me. Working as expected.
QA: Looking here
Will finish this Monday.
Created attachment 138448 [details] [review] Bug 27849: Koha::Token may access undefined C4::Context->userenv The _add_default_csrf_params internal function accesses C4::Context->userenv without checking that it has been defined. I think not all of the potential callers of it declare that they require a defined userenv, so we should test and provide defaults for required values if it is not defined, to avoid some "Can't use an undefined value as a HASH reference" HTTP 500 Internal Server Errors. To test: Do anything that requires a form with CSRF token, such as editing your details. Behaviour should be unchanged. To test the failure case, you would need some customised code that indirectly generates a CSRF token before setting the userenv up and I am not sure there is any in released Koha yet. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Looks good to me. Working as expected. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 138449 [details] [review] Bug 27849: (follow-up) Add test Compacting the code a bit too. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.06
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document it seems, marking resolved.
Thanks! Pushed to 21.11 for 21.11.12