When user inputs were appended directly to the barcode table, the values were not properly escaped, allowing potential XSS attacks.
Created attachment 171164 [details] Ignore
Created attachment 171165 [details] [review] Bug 37861: Fix XSS vulnerability by sanitizing user input in barcode append function.
Hi Artur, please file any potential security bugs/vulnerabilities in the 'Koha security' project. They will be fully published after testing and fixing.
Created attachment 171279 [details] [review] Bug 37861: Fix XSS vulnerability by sanitizing user input in barcode append function. Signed-off-by: Bo Gustavsson <bosse@gustavsson.one>
(In reply to Katrin Fischer from comment #3) > Hi Artur, please file any potential security bugs/vulnerabilities in the > 'Koha security' project. They will be fully published after testing and > fixing. Thank you, sorry for making you call me out more than once. I also wonder if I’ve done everything correctly so far, or if I need to do something more before my patch can get a QA.
I am a bit worried that a fix in JS might not be enough here. Should we not validate server side too?
No. The script injected only runs on the client who did the injection. It does send the text to the server, but the server only makes a check for a matching book in the database, nothing else. Maybe I should change the name of this bug to script injection instead of xss because it isn’t cross-site. If so, should I also remove the bug from Kona Security.
At a quick glance, it looks like this code could work, but we'll want to review it further (probably after Kohacon).
Pls QA
(In reply to Artur from comment #9) > Pls QA It's in the queue ;)
Thank you
Seems to work and yes it makes more sense now you explain it isnt XSS as you cant trigger someone else to run it. Good to stop it anyway. I'll QA it
Created attachment 172931 [details] [review] Bug 37861: Fix XSS vulnerability in barcode append function When user inputs were appended directly to the barcode table, the values were not properly escaped, allowing potential XSS attacks. This patch ensures that user inputs are sanitized and safely added to the DOM using .text() and .attr() methods to prevent script injection. To test: Enable the "SelfCheckInModule". Open the barcode input form. Enter a barcode with HTML or script tags. Without the patch, observe that the script is executed. Apply the patch. Repeat step 2. Verify that the input is escaped and no script execution occurs. Check that the barcode is properly appended to the table. Documentation: No updates required. Sponsored-by: KillerRabbitAos Signed-off-by: Bo Gustavsson <bosse@gustavsson.one> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
My patch also fixes the bug where you can submit empty books. That bug already has a pushed patch but this is (in my opinion) a more elegant solution. Avoiding adding extra event listeners.
Applied to 23.05.x-security for 23.05.16
When generating release notes, I was getting this error: root@9807bee652dc:/koha# koha-release v23.05.15..HEAD notes 23.05.16.000 <sponsor> does not match expected output for git check-mailmap at /release-tools/lib/Koha/Release.pm line 310. Figured out that it was because of this: Sponsored-by: KillerRabbitAos It needs to be on the same line, like this: Sponsored-by: KillerRabbitAos I fixed it on my branch but just letting other rmaints etc know :)
Applied to 23.11.x-security for v23.11.10
What does applied to mean? And how does security versions work. I’m sorry for being such a pain, I’m just getting the hang of Koha and have my ways of missing stuff in the documentation.
Created attachment 174312 [details] [review] New patch file that’s formatted correctly.
(In reply to Artur from comment #18) > What does applied to mean? And how does security versions work. I’m sorry > for being such a pain, I’m just getting the hang of Koha and have my ways of > missing stuff in the documentation. Hi Artur, Applied means we have applied it to our respective branches that we look after as release maintainers :) Hope that helps?