[Bug 7013] New: required format is not enforced for authorized values
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Bug #: 7013 Summary: required format is not enforced for authorized values Classification: Unclassified Change sponsored?: --- Product: Koha Version: master Platform: All OS/Version: All Status: NEW Severity: normal Priority: P5 Component: System Administration AssignedTo: paul.poulain@biblibre.com ReportedBy: gaetan.boisson@biblibre.com QAContact: koha-bugs@lists.koha-community.org CC: gmcharlt@gmail.com When creating a new authorized value in: Home › Administration › Authorized Values › New authorized value There is no control on what the user enters as the authorized value. However, entering a value which includes spaces or any special character(quotes, accents, etc.) will create problems at other places in the admin interface. Proper instructions are also required on the page (with the list of allowed characters) as well as in the manual. Here is an example of issue this can generate: use CCODE as the type used to limit branch transfer create CCODE values like "compact disc", "children's music" go to Home › Administration › Set Library Checkin and Transfer Policy tabs are not working because of the malformed CCODE value. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Sophie MEYNIEUX <sophie.meynieux@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |sophie.meynieux@biblibre.co | |m AssignedTo|paul.poulain@biblibre.com |adrien.saurat@biblibre.com -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #1 from Adrien SAURAT <adrien.saurat@biblibre.com> 2011-12-05 09:41:17 UTC --- Created attachment 6577 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=6577 proposed patch -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Adrien SAURAT <adrien.saurat@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P5 - low |PATCH-Sent Patch Status|--- |Needs Signoff --- Comment #2 from Adrien SAURAT <adrien.saurat@biblibre.com> 2011-12-05 09:42:18 UTC --- Two sentences created (a hint and an error message) in the proposed patch, to be translated. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #3 from Frère Sébastien Marie <semarie-koha@latrappe.fr> 2011-12-05 10:17:25 UTC --- Comment on attachment 6577 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=6577 proposed patch This patch is vulnerable to SQL injection. Please use placeholders in SQL query (using prepare/execute), instead of $variable. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #4 from Adrien SAURAT <adrien.saurat@biblibre.com> 2011-12-05 11:07:29 UTC --- I didn't change the SQL parts, they could be indeed enhanced but it would be better to make a specific bug about this, to make things clear. But thanks to your remark, I noticed a version problem :/ the script has probably been modified as I was working on this. I'll handle the conflicts and upload a new patch. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Adrien SAURAT <adrien.saurat@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #6577|0 |1 is obsolete| | --- Comment #5 from Adrien SAURAT <adrien.saurat@biblibre.com> 2011-12-05 13:17:32 UTC --- Created attachment 6579 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=6579 patch Diff show a lot of modified lines because the indentation of a few blocks changed, but I actually added a just few lines. What I did : - added the regex "$new_authorised_value =~ /^[a-zA-Z0-9\-_]+$/" to allow only letters, numbers, dash and underscore for a new value; - added a new hint next to the input box; - added a new error message when the user types an incorrect value. Also removed a few trailing spaces (they caused warnings when applying the patch). -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #6 from Chris Cormack <chris@bigballofwax.co.nz> 2011-12-05 15:47:27 UTC --- I agree with comment 3, this code is still vulnerable to sql injection. I will add a follow up, but I would encourage all developers to fix any bad coding practices like this that they say, as they change code. It is the perfect opportunity and really, there is no good reason to ever not use sql placeholders. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #7 from Chris Cormack <chris@bigballofwax.co.nz> 2011-12-05 15:49:00 UTC --- Created attachment 6582 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=6582 Bug 7013 : Fixing sql injection problems -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Needs Signoff |Failed QA --- Comment #8 from Owen Leonard <oleonard@myacpl.org> 2011-12-05 20:14:09 UTC --- I tried submitting an invalid value for a CCODE and did get an error message, but the error messages was on the Authorised Values home page (/cgi-bin/koha/admin/authorised_values.pl) rather than on the submission form for CCODE (/cgi-bin/koha/admin/authorised_values.pl?op=add_form&category=CCODE). The error messages should show on the same page from which the user submitted the invalid value, ideally with all the other values retained in the form so that they don't have to be re-typed. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #9 from Adrien SAURAT <adrien.saurat@biblibre.com> 2011-12-06 08:16:01 UTC --- @Chris: Ok! @Owen: Yes, to be consistent I used the same behaviour as another error message already implemented (when you try to use a value already in use). It would be better to do everything on the same page, I agree. I don't have time for this right now but I'll take a look asap. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Adrien SAURAT <adrien.saurat@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #6579|0 |1 is obsolete| | Attachment #6582|0 |1 is obsolete| | --- Comment #10 from Adrien SAURAT <adrien.saurat@biblibre.com> 2011-12-22 09:38:11 UTC --- Created attachment 6921 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=6921 proposed patch with client-side test I left the server-side controls (check if duplicate entry), but added a client-side control with javascript alert window, as seen in Patron categories GUI. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Adrien SAURAT <adrien.saurat@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Failed QA |Needs Signoff -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Adrien SAURAT <adrien.saurat@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Needs Signoff |--- --- Comment #11 from Adrien SAURAT <adrien.saurat@biblibre.com> 2011-12-22 10:41:54 UTC --- Well, in fact it seems that preventing spaces in every authorized value could cause more harm than good. Only in "ccode" should this format be really enforced. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #12 from Gaetan Boisson <gaetan.boisson@biblibre.com> 2011-12-22 10:47:46 UTC --- What i understood so far is that constraining the authorised value to a formated code can generate problems elsewhere, particularly if we want the value to be indexed and searchable. The problem could be shifted from the enforcement of a required format to the fix of the places where the interface is broken when an autorised value contains spaces or other problematic characters. Or as Adrien suggests, just enforce a format for CCODE. (But then what if other values break the interface? do we have other places where authorized values appear in tabs?) -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> 2011-12-22 10:50:23 UTC --- I am not sure I agree here. Authorized value codes are codes for internal use. To me it makes a lot of sense to restrict them to alphanumeric values and perhaps underscored. Display and search should use the description? Where do we need spaces? -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Laurence Lefaucheur <laurence.lefaucheur@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |laurence.lefaucheur@biblibr | |e.com --- Comment #14 from Laurence Lefaucheur <laurence.lefaucheur@biblibre.com> 2011-12-22 11:39:53 UTC --- Unless I mistake, Zebra search currently does not use description but AV code. We set AV=description for libraries that want to search on indexes like location or specific indexes in full words, as patrons never know the codes. Enforcing a code for these values will imply a loss of search functionalities, imho. Maybe CCODE should be a specific AV, as it is involved in specific rules or parameters, but I'm not sure I agree to enforce a code for all AV. Unless zebra is able to search in description in a relatively short delay ? Solr already do that. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> 2011-12-22 11:44:26 UTC --- I can see why it would make sense to do that, but I don't think we should use internal codes like this. The search should be fixed in this case. We have search options in advanced search to translate codes into descriptions. And we do the same when searching for branches, in advanced search and in facets. Display description, search for code. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|--- |Failed QA -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Adrien SAURAT <adrien.saurat@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |In Discussion -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> 2012-01-17 09:05:00 UTC --- Now as this has moved into 'In Discussion' I think it will take a while to be resolved. I think we should fix the security problem found asap. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Adrien SAURAT <adrien.saurat@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |7450 -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #17 from Adrien SAURAT <adrien.saurat@biblibre.com> 2012-01-17 11:07:57 UTC --- A patch fixing the security issue (missing placeholders) has been proposed in bug 7450. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.poulain@biblibre.com --- Comment #18 from Paul Poulain <paul.poulain@biblibre.com> 2012-02-10 14:00:40 UTC --- In discussion = it's quite unclear to me how to start the discussion. Could one of you start the discussion wiki page & the thread on koha-devel & koha mailing list ? -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #19 from Paul Poulain <paul.poulain@biblibre.com> 2012-02-10 14:01:21 UTC --- the wiki link to start the discussion = http://wiki.koha-community.org/wiki/Bug_and_Enhancement_Discussion -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Sophie MEYNIEUX <sophie.meynieux@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|adrien.saurat@biblibre.com |christophe.croullebois@bibl | |ibre.com -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 --- Comment #20 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think in order to fix this properly we need to fix how MANUAL_INV works. It uses the code as description and the description for the amount. We could use the code as a real code and then the description as description... and the opac description for the amount. But it would still be bending something to do what it was not supposed to do in the first place. And do we still need to fix the security issue?? -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |oleonard@myacpl.org --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 10237 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7013 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org