Bug 17720 - CSRF token is not generated correctly
Summary: CSRF token is not generated correctly
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low blocker (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 17096
Blocks:
  Show dependency treegraph
 
Reported: 2016-12-05 08:00 UTC by Jonathan Druart
Modified: 2017-06-14 22:17 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17720: CSRF - Handle unicode characters (13.10 KB, patch)
2016-12-05 08:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17720: CSRF - Handle unicode characters (13.17 KB, patch)
2016-12-05 12:59 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17720: CSRF - Handle unicode characters (12.49 KB, patch)
2016-12-05 13:37 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-12-05 08:00:34 UTC
The CSRF token is not generated correctly if the DB password contain unicode characters.

Indeed md5_base64 explodes with
  Wide character in subroutine entry at /usr/share/perl5/Digest/HMAC.pm line 63.
Comment 1 Jonathan Druart 2016-12-05 08:24:15 UTC
Created attachment 57949 [details] [review]
Bug 17720: CSRF - Handle unicode characters

From the pod of Digest::MD5:
"""
Since the MD5 algorithm is only defined for strings of bytes, it can not
be used on strings that contains chars with ordinal number above 255
(Unicode strings). The MD5 functions and methods will croak if you try
to feed them such input data.
What you can do is calculate the MD5 checksum of the UTF-8
representation of such strings.
"""

Test plan:
- Set a MySQL/MariaDB password with unicode characters:
  UPDATE user SET password=PASSWORD('❤') WHERE USER='koha_kohadev';
  FLUSH PRIVILEGES
- Update your $KOHA_CONF file
- Restart Memcached
- Hit the files modified by this patch

=> Without this patch, you will get a software error (with "Wide
character in subroutine entry" in the logs).
=> With this patch, everything will go fine
Comment 2 Josef Moravec 2016-12-05 12:59:18 UTC
Created attachment 57957 [details] [review]
[SIGNED-OFF] Bug 17720: CSRF - Handle unicode characters

From the pod of Digest::MD5:
"""
Since the MD5 algorithm is only defined for strings of bytes, it can not
be used on strings that contains chars with ordinal number above 255
(Unicode strings). The MD5 functions and methods will croak if you try
to feed them such input data.
What you can do is calculate the MD5 checksum of the UTF-8
representation of such strings.
"""

Test plan:
- Set a MySQL/MariaDB password with unicode characters:
  UPDATE user SET password=PASSWORD('❤') WHERE USER='koha_kohadev';
  FLUSH PRIVILEGES
- Update your $KOHA_CONF file
- Restart Memcached
- Hit the files modified by this patch

=> Without this patch, you will get a software error (with "Wide
character in subroutine entry" in the logs).
=> With this patch, everything will go fine

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 3 Tomás Cohen Arazi 2016-12-05 13:37:35 UTC
Created attachment 57967 [details] [review]
Bug 17720: CSRF - Handle unicode characters

From the pod of Digest::MD5:
"""
Since the MD5 algorithm is only defined for strings of bytes, it can not
be used on strings that contains chars with ordinal number above 255
(Unicode strings). The MD5 functions and methods will croak if you try
to feed them such input data.
What you can do is calculate the MD5 checksum of the UTF-8
representation of such strings.
"""

Test plan:
- Set a MySQL/MariaDB password with unicode characters:
  UPDATE user SET password=PASSWORD('❤') WHERE USER='koha_kohadev';
  FLUSH PRIVILEGES
- Update your $KOHA_CONF file
- Restart Memcached
- Hit the files modified by this patch

=> Without this patch, you will get a software error (with "Wide
character in subroutine entry" in the logs).
=> With this patch, everything will go fine

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Edit: removed debugging leftover
Comment 4 Kyle M Hall 2016-12-05 15:20:28 UTC
Pushed to master for 17.05, thanks Jonathan!
Comment 5 Katrin Fischer 2016-12-05 20:28:07 UTC
This patch has been pushed to 16.11.x, will be in 16.11.01.