Bug 42025

Summary: Error handling in tools/upload is broken
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: ToolsAssignee: Bugs List <koha-bugs>
Status: NEW --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: dcook, m.de.rooy
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41580
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Crowdfunding committed: 0
Crowdfunding contact: Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 13618    
Bug Blocks:    

Description Jonathan Druart 2026-03-09 09:55:30 UTC
On the following commit we add the html filter to msg:

  commit dcd1f5d48c758aee17b6c6f069c6146b42efe117
  Date:   Fri Jan 23 13:18:54 2015 +0100
  Bug 13618: Add html filters to all the variables

-                ShowAlerts( [% msg %] );
+                ShowAlerts( [% msg | html %] );

msg is supposed to be a JSON encoded string:

tools/upload.pl
118     my $msg =
119           $delete ? JSON::to_json( { $fn => { code => ALERT_DELETED } } )
120         : $id     ? JSON::to_json( { $fn || $id, { code => ERR_NOT_DELETED } } )
121         :           '';

So we could use To.json in the template and remove the to_json from the controller.

BUT...
There are a couple places in the controller where we send back what we send:
38 my $msg            = $input->param('msg');
76         msg            => $msg,

Which would lead to XSS if we remove the filtering.

This script receives msg in parameter when sent from SubmitMe
462         function SubmitMe(op, id, msg ) {
465             $("#submitter #msg").val( msg );

coming from:
398         function cbUpload( status, fileid, err ) {
399             $('#fileToUpload').prop('disabled', false);
400             if( status=='done' ) {
401                 var e = err? JSON.stringify(err): '';
402                 SubmitMe( 'search', fileid, e );

Coming from
364         function StartUpload() {
387             xhr= AjaxUpload( $('#fileToUpload'), $('#fileuploadprogress'), xtra, cbUpload );

Which does...:
koha-tmpl/intranet-tmpl/prog/js/file-upload.js
 1 function AjaxUpload(input, progressbar, xtra, callback) {

10     var url = "/cgi-bin/koha/tools/upload-file.pl?" + xtra;

19     xhr.onload = function (e) {
20         var data = JSON.parse(xhr.responseText);
21         if (data.status == "done") {
22             progressbar.val(100);
23             progressbar.next(".fileuploadpercent").text("100");
24         }
25         callback(data.status, data.fileid, data.errors);
26     };

tools/upload-file.pl
 55     send_reply( 'failed', undef, $upload ? $upload->err : undef );

 63 sub send_reply {    # response will be sent back as JSON
 64     my ( $upload_status, $data, $error ) = @_;
 65     my $reply = CGI->new("");
 66     print $reply->header( -type => 'text/html', -charset => 'UTF-8' );
 67     print JSON::encode_json(
 68         {
 69             status => $upload_status,
 70             fileid => $data,
 71             errors => $error,
 72         }
 73     );

Koha/Uploader.pm
154 sub err {
155     my ($self) = @_;                                                                                  
156     my $err;
157     foreach my $f ( keys %{ $self->{files} } ) {                               
158         my $e = $self->{files}->{$f}->{errcode};                                                      
159         $err->{$f}->{code} = $e if $e;                                  
160     }
161     return $err;
162 }

All this is VERY messy...