File Path Back Slashes

Question

I have put together a fairly basic local database driven website using PHP, Appache, and PHPMyAdmin. The site allows users to upload cad detail files in two formats. Along with the file paths, they can also upload the name of the file, the related service group, as well as the detail ID.

Basically everything is working smoothly except for a small issue when uploading a file from the upload page. Everything submits to the database just fine but the links to the files are missing the necessary backslashes.

I can however go into the database itself and enter the backslashes into the file path.

Form:

<form method="post" action="add.php">
 <table border="0">


 <tr><td>Detail ID: </td><td><input type="number" name="id" /></td></tr>

 <tr><td>Detail Name: </td><td><input type="text" name="name" /></td></tr>

 <tr><td>Service Group: </td><td><input type="text" name="service" /></td></tr>

 <tr><td>PDF: </td><td><input type="file" name="pdf" enctype="multipart/form-data"/></td></tr>

 <tr><td>DWG: </td><td><input type="file" name="dwg"/></td></tr>

 <tr><td></td><td><input type="submit" value="Submit" />
    <input type="reset" value="Reset" /></td></tr>

 </table>
 </form>

PHP:

$con=mysql_connect("localhost","root","");
 /* Select the database */
 mysql_select_db("hrg_test");

 /* Store the values submitted by form in variable */
 $id=$_POST['id'];
 $name=$_POST['name'];
 $service=$_POST['service'];
 $pdf=$_POST['pdf'];
 $dwg=$_POST['dwg'];


 /* Write a query to insert details into database */
 $insert_detail=mysql_query("INSERT INTO hrg_test (id, name, service, pdf, dwg) VALUES ('$id',      '$name',     '$service',     '$pdf',     '$dwg')");

 if($insert_detail)
 { echo "Detail Succesfully Added! <br /><br /><a href='add.html'>Add Another Detail</a>"; }
 else
 { echo "Error".mysql_error(); }

 /* closing the if else statements */

 mysql_close($con);
 ?>

I've read about the magic quotes and stripslashes, although i'm not sure how to tie them in if that's the issue. Maybe i am just going about it the wrong way.

Any help would be greatly appreciated.

Answer 1

I would imagine that you could get this working by using these on each of your input POST variable assignments.

$pdf=mysql_real_escape_string($_POST['pdf']);

However. The problems here are deeper. It may well only be a local script for now. We've all been there. It's surprising how quickly things can change, and there may not always be time to refactor (The boss loves the prototype and wants the finished app yesterday), or somebody copies your code (maybe you?) for use in a more public environment. Even if this script were behind an "admin only" area, a malicious coder (or disgruntled admin?) could gain access to the admin system and then would be able to upload all kinds of nasty things and do unsavory things to your database! Trust no one.

Here is a bit of a stab at outlining a few of the problems and how it could be improved for less than zero cost in terms of effort.

1. MySQL API. This extension is not recommended for writing new code. Instead, either the mysqli or PDO_MySQL
2. This code is vulnerable to database injection attacks. You need to escape your data. This means making the data safe to insert into a database (escaping ' characters for instance). You could use mysql_real_escape_string() but this can still leave security holes, and allows you to the opportunity to forget to escape. But if we were to choose the PDO extension then you can use parameterised queries and all your problems will be magically solved.
3. Accepting user uploads is always risky. The more you can restrict it the better. If you know you only want to accept a certain type of file then make sure you check the file type. If possible, move the file to somewhere that cannot be accessed via an HTTP request. If it has to be accessible over HTTP, then try to rename the file to something that cannot be easily guessed. Perhaps a random string of 30 characters or so. This means that malicious coders cannot upload a naughty PHP script named 'bad_things.php', and simply visit 'www.example.com/uploads/bad_things.php' to have the script run on your server. You don't seem to be saving uploaded files anywhere so I haven't included an example. There are infinite examples out there: here's something that looks pretty reasonable

Here is some example code that is not just secure, but also much simpler (and shorter, not that that matters one jot.)

---

$dbh = new PDO('mysql:host=localhost;dbname=hrg_test', 'root', '');
$stmt = $dbh->prepare("INSERT INTO hrg_test (id, name, service, pdf, dwg) VALUES(?,?,?,?,?)");
$result = $stmt->execute(array($_POST['id'],$_POST['name'],$_POST['service'],$_POST['pdf'],$_POST['dwg']));

if($result){ 
    echo "Detail Succesfully Added! <br /><br /><a href='add.html'>Add Another Detail</a>"; 
}
else { 
    var_dump($stmt->errorInfo());
}

Answer 2

I'll ignore the security issues since this is on a local site (nobody ever reads that and ingests what it means apparently). You should look at the newer methods like mysqli or PDO, but try wrapping the values with mysql_real_escape_string(). That should escape the slashes and allow them to be stored if that is the problem.

On a side note, I also develop for a closed down corporate intranet. Even though I know how, my boss begs me to forego the needless security in many cases because the only access is by staff.

Pointing out that there are security issues is fine if you're also going to answer the question... if not, you're just wasting yours and everyone else's time.

Answer 3

Another thing to watch is the use of the double quotes. I noticed your echo statements used double quotes (") which cause the string inside the quotes to be preprocessed (variables are expanded, etc...). If you manipulate any data, and place paths in strings within double quotes, the backslashes will turn the following character into a literal, and cause your backslash to disappear.

For instance:

$UploadPath = "New\Drawings";
$NewPath = "C:\UPLOADS\$UploadPath";

In this case, the $NewPath gets converted to be 'C:UPLOADSNewDrawings' as each backslash makes the character after it a literal. Using single quotes (') will prevent this, or double backslashes.

Answer 4

My point Joe was not that security isn't important, just that it isn't necessary for you to critique security in code on a site like SO. In fact, I would love to see a site like SO ban it altogether. If I am asking about security, fine, but if I post a code snippet (that I probably wrote just to show an example of what I mean) I shouldn't have to deal with a barrage of comments ranging from "Here are 37 links about security" to "Have fun getting your database pwned" or whatever snappy comment someone wants to leave.

The objective, should always be to answer the OP's question. It's almost like the minute someone wraps their head around security principles, they think they need to troll the internet to save bad coders from making mistakes. I honestly find it to be completely misplaced energy since the VAST majority of people respond with "I know, this is just an example for purposes of posting".

Not to mention, in your example... you say "much simpler?" Including the curly brace lines, your snippet is 11 lines of code and uses a syntax people should learn, but may not be used to yet. The OP posted 14 lines of code (5 of which were making post variables local which was unneeded and would take his down to 9 lines of code. What about it is "Much easier"? I hate to be the devil's advocate here, but saying PDO/mysqli are simpler is an opinion, they are better for sure, but simpler is a toss up.

Your answer would have been great had security been the focus of the question... I would honestly consider what you did a hijack.