d2jsp
Log InRegister
d2jsp Forums > Off-Topic > Computers & IT > Programming & Development > Javascript/php File Upload Incomplete
Add Reply New Topic New Poll
Member
Posts: 21,861
Joined: Dec 13 2010
Gold: 0.91
May 5 2016 06:54am
Greetings gentlemen,

I got the following problem:

If I upload a file, the uploaded file is imcomplete,
e.g I upload a 416 kb .csv file with ~3000 lines and the uploaded file is only 235 lines long.

The servers settings are fine (memory limit, max_post_size, max_uplaod_size)

here's some code:


whole Upload.php:

Code
<?php
/*
* Name: PHP: upload
*
* Description:
* - handles the uploaded .csv file which was sent by the frontend. </br>
* - starts the session, the session id defines the filename of the .csv </br>
* - executes function preprocess (PHP: preprocessing methods) </br>
* - executes function initOptions (PHP: initOptions methods) </br>
* - executes start (PHP: Server Class) </br>
* - executes printData (PHP: Options Class)
*/
// to show php errors..
error_reporting(E_ALL);
ini_set('display_errors', 'On');
include_once("preprocessing.php");
include_once("initOptions.php");
include_once("searchOptions.php");
include_once("config.php");
include_once("server.php");

$output_dir = $PATH_TO_WEBAPP . "/uploads/";
if(isset($_FILES["file"])) {
@session_start();
if ($_FILES["file"]["error"] > 0) {
echo "Error: " . $_FILES["file"]["error"] . "<br>";
}
else {
//move the uploaded file to uploads folder;
move_uploaded_file($_FILES["file"]["tmp_name"], $output_dir. session_id().".csv.orig");
}
$_SESSION["shared"] = false;
$_SESSION["db"] = session_id();
$filename = $output_dir. session_id().".csv.orig";
preprocess($filename);
initOptions();
Server::start();
Options::printData();
}
return;
?>






javascript snippet:
it is using this upload script: https://github.com/michaelcbrook/simpleUpload.js

Code

$(document).on('change', '.btn-file :file', function() {
var input = $(this),
numFiles = input.get(0).files ? input.get(0).files.length : 1,
label = input.val().replace(/\\/g, '/').replace(/.*\//, '');
input.trigger('fileselect', [numFiles, label]);
input.simpleUpload("php/upload.php", {
start: function() {
var button1 = input.parents('.btn-file').find('.txt-bfr-upld');
var button2 = input.parents('.btn-file').find('.txt-aft-upld');
button1.toggleClass("hide");
button2.toggleClass("hide");
},
progress: function(progress) {
//received progress
$('.btn-progress').text("Progress: " + Math.round(progress) + "%");
},
success: function(data){
//upload successful
$('.btn-progress').text("Success!");
$('.view-bfr-upld').toggleClass('hide')
$('.view-aft-upld').toggleClass('hide')
updateAllBoxes("", 5);
createCheckboxes(data);
shared = false;
$('#settings > span').css("opacity", "1");
$('#settings > span').removeAttr("title");
},
});
});


anyone got an Idea why this happens?
thx in advance
Member
Posts: 13,425
Joined: Sep 29 2007
Gold: 0.00
Warn: 20%
May 5 2016 02:33pm
Idk what the problem is but I am a bit weary that your upload script does not actually check for a valid session before uploading the file. This might lead to an attacker falsifying a session ID to generate a predictable file name and uploading a web shell compromising your www-user on your server.
Member
Posts: 21,861
Joined: Dec 13 2010
Gold: 0.91
May 5 2016 02:38pm
Quote (AbDuCt @ 5 May 2016 22:33)
Idk what the problem is but I am a bit weary that your upload script does not actually check for a valid session before uploading the file. This might lead to an attacker falsifying a session ID to generate a predictable file name and uploading a web shell compromising your www-user on your server.


hi, thanks for the answer.

I figured out that not the upload was responsible for this,

the preprocessing we perform had a weird bug with "iconv"

when we converted the file to UTF8 the method had an internal limit how much it can deal with if you want to do it inplace,
directing the output to a new file & renaming it afterwards fixed the error.

for smaller files it worked fine, that's why i didn't consider it at all :lol:

anyway, all works fine now^^


the security issue i should fix for sure tho^^

did you mean probably something like this:

Code
function isValidSessionId($session_id)
{
return !empty($session_id) && preg_match('/^[a-zA-Z0-9]{26,40}$/', $session_id);
}


This post was edited by Breee on May 5 2016 02:46pm
Member
Posts: 13,425
Joined: Sep 29 2007
Gold: 0.00
Warn: 20%
May 5 2016 04:21pm
Quote (Breee @ May 5 2016 04:38pm)
Code
function isValidSessionId($session_id)
{
return !empty($session_id) && preg_match('/^[a-zA-Z0-9]{26,40}$/', $session_id);
}


Well the session is something given to the user from the server so there is no need to check the session for validity since if the session doesn't exist it will automatically create a new valid one. I misspoke in my previous post.

The part that is concerning is that depending on the implementation the user knows their session ID and can predict the file name that will be created by your script. You will need to find a way to verify that the data the user is uploading is in fact CSV (hard), or transfer the files to a non executable directory owned by the www-user (easy).

So rather than storing the files for instance in /var/www/website.org/uploads/ which a user can navigate to http://website.org/uploads/filename.sessionid.csv.orig to execute their malicious files, to store them in a directory not in the www root. For instance in /var/uploads/website.org/ which may have the flags of 0750. Since the web server can't actually execute files outside it's web directory the problem is partly solved. Depending on the implementation of the file fetching script you can include a whole host of new attacks like local file inclusion (LFI) and directory traversals.

The real only solution is to combine lots of techniques and to never reveal the filename of the file stored to the user.

More can be read here: https://web.archive.org/web/20140927111708/http://blog.insicdesigns.com/2009/01/secure-file-upload-in-php-web-applications

This covers in depth the attacks on all the common upload scripts people make (including yours) and why they are flawed, how to exploit them, and how to fix them.

This post was edited by AbDuCt on May 5 2016 04:29pm
Member
Posts: 21,861
Joined: Dec 13 2010
Gold: 0.91
May 5 2016 04:39pm
Quote (AbDuCt @ 6 May 2016 00:21)
Well the session is something given to the user from the server so there is no need to check the session for validity since if the session doesn't exist it will automatically create a new valid one. I misspoke in my previous post.

The part that is concerning is that depending on the implementation the user knows their session ID and can predict the file name that will be created by your script. You will need to find a way to verify that the data the user is uploading is in fact CSV (hard), or transfer the files to a non executable directory owned by the www-user (easy).

So rather than storing the files for instance in /var/www/website.org/uploads/ which a user can navigate to http://website.org/uploads/filename.sessionid.csv.orig to execute their malicious files, to store them in a directory not in the www root. For instance in /var/uploads/website.org/ which may have the flags of 0750. Since the web server can't actually execute files outside it's web directory the problem is partly solved. Depending on the implementation of the file fetching script you can include a whole host of new attacks like local file inclusion (LFI).

The real only solution is to combine lots of techniques and to never reveal the filename of the file stored to the user.

More can be read here: https://web.archive.org/web/20140927111708/http://blog.insicdesigns.com/2009/01/secure-file-upload-in-php-web-applications

This covers in depth the attacks on all the common upload scripts people make (including yours) and why they are flawed, how to exploit them, and how to fix them.



oh, that's pretty nice of you^^
i'll take a look on it.

Go Back To Programming & Development Topic List
Add Reply New Topic New Poll