---- Reported by rick@praxis.com.au 2008-12-05 02:34:55 ---- The values of input fields, the names and values in tags in general, and the passing of data via HTML all require HTML escaping, i.e. translating < to < and " to ". Here is a concrete example of the HTML escape problem. 1. In the Modify Patron script members/memberentry.pl, the borrower attributes can be modified. And in the Admin area the attribute types (or codes) can be defined. 2. Create an attribute whose code is this: REF "B" 3. The attribute can be created but it cannot be modified, because the HTML for the modification script says this: <input ... value="REF "B""> The browser interprets the value as "REF ", not 'REF "B"' 4. Once that patron attribute code is in the system, all hell can break lose. The attributes for that patron can no longer be edited. The error message is something like the following in Koha/3.0: The following fields are wrong. Please fix them. * The attribute value REF /slarty is already is use by another patron record. The script should have been given REF "8" as the field name, but got "REF" instead. 5. The fix is to add the escape parameter to the TMPL_VAR tag: <input ... value="<!--TMPL_VAR Name="something" escape="html" -->" /> 6. Other tags present the same problem: <option> esp. the value= attribute of the tag In general, look for the following construct and add the escape="html" attribute to the TMPL_VAR tag: <whatever ... value="<!--TMPL_VAR Name="something"--> /> ---- Additional Comments From cjaeger@dce.k12.wi.us 2009-04-19 22:48:24 ---- Created an attachment Patch for categorie.tmpl Patch for categorie.tmpl Many templates still need to be updated for this bug. ---- Additional Comments From cjaeger@dce.k12.wi.us 2009-04-20 16:16:47 ---- Created an attachment Corrected patch for categorie.pl Correct html and url escaping in categorie.pl template. Use this patch and not the previous. --- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:57 UTC --- This bug was previously known as _bug_ 2847 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=2847 Imported an attachment (id=887) Imported an attachment (id=888) Actual time not defined. Setting to 0.0 The original submitter of attachment 887 [details] [review] is unknown. Reassigning to the person who moved it here: chris@bigballofwax.co.nz. The original submitter of attachment 888 [details] is unknown. Reassigning to the person who moved it here: chris@bigballofwax.co.nz.
Created attachment 6294 [details] [review] Proposed fix Fixes for output in a couple of acquisitions templates where user-generated data should be escaped. This instances were found by creating a vendor name like "Baker & Taylor" and finding that the ampersand was not escaped, causing validation errors. This patch also consolidates multiple <script> blocks which do not need to be separate and corrects a couple of unclosed <input> tags.
Created attachment 6299 [details] [review] Incremental fix for Bug 2847, Use HTML escape in templates where appropriate Fixes for output in a couple of acquisitions templates where user-generated data should be escaped. This instances were found by creating a vendor name like "Baker & Taylor" and finding that the ampersand was not escaped, causing validation errors. This patch also consolidates multiple <script> blocks which do not need to be separate and corrects a couple of unclosed <input> tags. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
QA comment: I'm not fan of mixing things in patches (html/js), but it's small & announced, so passed QA Patch pushed, please test
This patch does not apply cleanly to the 3.4.x branch. If it should, please rebase over that branch and attach the corrected path to this bug report. cnighs@erasmus:~/koha.3.2.test (3.4.x)$ ./get_commits.pl -b 3.4.x -l 35234b478a Shall I apply the bugfix(s) for 2847? (Y/n)y Automatic cherry-pick failed. After resolving the conflicts, mark the corrected paths with 'git add <paths>' or 'git rm <paths>' and commit the result with: git commit -c bfe06ef399da831e5602784fbe54cea6dfc1ab65
is it just a problem on 3.4 ? it's OK for 3.6 ? In this case, I think we can stay without this fix in 3.4 : it's a security issue, I agree, but: * it's staff related, so, to exploit such a bug, one would first need to have a valid login, so, the risk is low according to me. * 3.4 EOL is probably close
(In reply to comment #5) > is it just a problem on 3.4 ? it's OK for 3.6 ? > In this case, I think we can stay without this fix in 3.4 : it's a security > issue, I agree, but: > * it's staff related, so, to exploit such a bug, one would first need to have a > valid login, so, the risk is low according to me. IMHO any security issue should be backported into any currently maintained branch. > * 3.4 EOL is probably close Not as close as we might imagine. As long as commits will apply, I plan on keeping 3.4.x alive until we reach a year from 3.6.x release.
Created attachment 6380 [details] [review] Proposed fix for 3.4.x
Included in the 3.6 branch prior to 3.6.4.