[Bug 38523] New: Simple quote in ILL standard form JS can break translations
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 Bug ID: 38523 Summary: Simple quote in ILL standard form JS can break translations Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: I18N/L10N Assignee: koha-bugs@lists.koha-community.org Reporter: victor@tuxayo.net QA Contact: testopia@bugs.koha-community.org CC: f.demians@tamil.fr, jonathan.druart@gmail.com Follow-up of Bug 36221. In Koha/ILL/Backend/shared-includes/shared.js '" name="custom_key" placeholder="'+__('key')+'">' + ' ' + '<input type="text" id="custom-value" name="custom_value" class="' + ( opac ? 'form-control input-fluid custom-field-input' : '') +'" placeholder="'+__('value')+'"> ' + '<button type="button" class="btn btn-danger btn-sm ' + 'delete-new-field">' + '<span class="fa fa-trash-can"></span> ' + __('Delete') + [...] $('#custom-warning').text(__("The name '%s' is not permitted").format(val)).show(); ------- This will need to be tested once bug 38340 has patches. For now stuff is not translatable but once it is, it's expected the JS can break if the translated string contains a simple quote. ------- xt/single_quotes.t doesn't catch it because the test only searches in koha-tmpl/opac-tmpl/bootstrap/en/ koha-tmpl/intranet-tmpl/prog/en/ -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |36221 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36221 [Bug 36221] Improve styling of Standard backend create OPAC form -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|major |normal CC| |pedro.amorim@ptfs-europe.co | |m -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |38340 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38340 [Bug 38340] Translatability - Standard form include files are not translatable -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 Bug 38523 depends on bug 38340, which changed state. Bug 38340 Summary: Translatability - Standard form include files are not translatable https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38340 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 Bug 38523 depends on bug 38340, which changed state. Bug 38340 Summary: Translatability - Standard form include files are not translatable https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38340 What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|FIXED |--- -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 Bug 38523 depends on bug 38340, which changed state. Bug 38340 Summary: Translatability - Standard form include files are not translatable https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38340 What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #1 from David Cook <dcook@prosentient.com.au> --- It looks like this has been moved into these files: ./koha-tmpl/opac-tmpl/bootstrap/en/includes/ill/backends/Standard/shared/shared.js ./koha-tmpl/intranet-tmpl/prog/en/includes/ill/backends/Standard/shared/shared.js And one day we really need to forbid creating HTML from Javascript strings. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- That said, what's the expected problem here? A single quote shouldn't be an issue since the HTML is encased in double quotes, but a double quote could be since that would create invalid HTML. Unless the problem is with __() specifically? It's been a little while since I looked at that specifically. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- Admittedly, I'm not super familiar with the entire translation process in Koha start to finish. Here's some steps I took: sudo apt-get install koha-l10n --reinstall ./debian/scripts/koha-translate -i fr-FR -d kohadev vi koha-tmpl/intranet-tmpl/prog/fr-FR/js/locale_data.js The "Delete" translation probably wouldn't cause syntax errors, but it could be used to inject XSS, although in theory the Translation Manager should catch that? It looks like "key" and "value" aren't currently translated... but I can test it anyway by manually editing locale_data.js. We'll inject this string into locale_data.js: "key":[null,"\"cle\""], After we install fr-FR, change to French, enable ILLModule, and go to http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl?method=create&backend=Standard we click on "Nouvelle demande de PEB", and we click on "Ajouter un nouveau champ". Et voilà... no placeholder text because we've accidentally generated <input type="text" class="custom-name " name="custom_key" placeholder="" cle""=""> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 --- Comment #4 from David Cook <dcook@prosentient.com.au> --- Now this is easy to avoid by using Javascript like document.createElement() to create the HTML Element Javascript object. Then to set the placeholder we can just do this: custom_name.placeholder = __("key"); By using the browser web APIs and avoiding Javascript string manipulation, we set the placeholder more securely and more robustly. -- We also don't even need jQuery to do this. Javascript has advanced so much that this can easily be done in regular Javascript, which is easier to maintain in the long-run as well. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Simple quote in ILL |Unexpected characters in |standard form JS can break |ILL standard form JS can |translations |break translations -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 Bug 38523 depends on bug 36221, which changed state. Bug 36221 Summary: Improve styling of Standard backend create OPAC form https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36221 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38523 --- Comment #5 from David Cook <dcook@prosentient.com.au> --- At some point maybe we need a QA script to forbid string concatenation in JS... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org