http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6874
--- Comment #19 from Frère Sébastien Marie <[email protected]> --- Hi, I only do a "visual review" of the code, but just some suggestions... In "C4/UploadedFiles.pm" - line 147: > if( -f $file_path ) { > warn "Id $id not in database, but present in filesystem, do nothing"; > return; > } Use "-e" instead of "-f": the difference is when $file_path is an existing link. -f say "not a file", but -e "a node exists" (may be a directory, a plain file, a socket, a pipe, a special block device, a symbolic link...) - line 153: > unless(open $out_fh, '>', $file_path) { > warn "Failed to open file '$file_path': $!"; > return; > } a race condition is possible between the check of file existence and the open of file. You could use sysopen function instead of file-check + open. > use IO::Handle; > unless( sysopen($out_fh, $path, O_WRONLY|O_CREAT|O_EXCL) ) { > warn "Failed to open file '$file_path': $!"; > return; > } note: the file is created only if don't exist, due to O_CREAT|O_EXCL. There is not time between check and open because all is done by system kernel. In ".../cataloguing/value_builder/upload.tt" (and others templates): please use "html" filter. It is (generally) always to use it, a prevent XSS (here only "uploaded_file" seems to be build with user-input) > The file [% uploaded_file | html %] has been successfully uploaded. In the syspref message, perhaps it should be added that the uploadPath MUST NOT be a public directory, accessible from the webserver (because else, the server could be exposed to arbitrary code execution by evil uploader). Else, the patch seems promising. Thanks -- You are receiving this mail because: You are the QA Contact for the bug. You are watching all bug changes.
_______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
