Steven Posted March 29, 2014 Share Posted March 29, 2014 (edited) After my crash course in PHP/SQL, my brain is a bit fried. I am sure I am missing some security methods in my scripts, and would appreciate the help of some trained eyes. On my "formJobs.php" page, there are three inputs: "Client", "Date", and "Description". The first two inputs are a dropdown list and a datepicker widget, so the user (I'm assuming?) doesn't really have any option or way to submit any invalid data between those first two inputs. They can only select clients already in the "Clients" table, or pick a valid date from a valid calendar pop-out. So I'm not worried about validating and sanitizing those entries, as they should be valid, in theory, because the user has no ability to tinker with it. But, with the "Description" input, which is a simple textbox that allows the user to give a brief description of the job, is wide open right now. The only thing I have in place is a snippet that makes sure a description is filled out. Here's my "formJobs.php" page: <?php require('includes/config.php'); ?><!doctype html><html lang="en"><head> <meta charset="utf-8"> <title>Modern Office | Management System</title> <link rel="stylesheet" href="css/style.css"> <link rel="stylesheet" href="css/foundation.css"> <link rel="stylesheet" href="http://ajax.googleapis.com/ajax/libs/jqueryui/1.10.1/themes/base/minified/jquery-ui.min.css" type="text/css" /> <script type="text/javascript" src="//ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js"></script> <script type="text/javascript" src="http://code.jquery.com/ui/1.10.1/jquery-ui.min.js"></script> <script type="text/javascript" src="js/formhint.js"></script> <script type="text/javascript"> $(document).ready(function(){ // Focus auto-focus fields $('.auto-focus:first').focus(); // Initialize focus-glow fields $('INPUT.focus-glow, TEXTAREA.focus-glow').focus(function(){ if($(this).val() == $(this).attr('title')){ $(this).val(''); $(this).removeClass('focus-glow'); } }); $('INPUT.focus-glow, TEXTAREA.focus-glow').blur(function(){ if($(this).val() == '' && $(this).attr('title') != ''){ $(this).val($(this).attr('title')); $(this).addClass('focus-glow'); } }); $('INPUT.focus-glow, TEXTAREA.focus-glow').each(function(){ if($(this).attr('title') == ''){ return; } if($(this).val() == ''){ $(this).val($(this).attr('title')); } else { $(this).removeClass('focus-glow'); } }); }); </script> <script type="text/javascript"> // The following script adds auto-complete functionality // to the client name field, pulling clients from the DB $(function() { $(".auto").autocomplete({ source: "search.php", minLength: 1 }); }); </script> <script type="text/javascript"> // This script allows the jQuery Datepicker // widget to be used for the three date fields $(function() { $( ".datepicker" ).datepicker({ dateFormat: 'yy-mm-dd' }); }); </script></head><body><div class="row"><div class="large-12 columns"> <?php include ('includes/header.php'); ?> <?php include ('includes/navigation.php'); ?></div> <!-- / columns --></div> <!-- / row --><div class="row"><div class="large-12 columns"> <h2>Add a new job</h2></div> <!-- / columns --></div> <!-- / row --><div class="row"><div class="large-8 columns large-centered"> <div class="panel"> <?php// Check for form submissionif ($_SERVER['REQUEST_METHOD'] == 'POST') { // Initialize an error array $errors = array(); // Check for a client entry if (empty($_POST['clientid'])) { $errors[] = 'You forgot to enter a client'; } else { $clientid = ($_POST['clientid']); } // Check for a date entry if (empty($_POST['datein'])) { $errors[] = 'You forgot to enter the date'; } else { $datein = ($_POST['datein']); } // Check for a description entry if (empty($_POST['description'])) { $errors[] = 'You forgot to enter a description'; } else { $description = trim($_POST['description']); } // If it's all good... if (empty($errors)) { // ...send form to the database // Make the query $q = 'INSERT INTO jobs (clientid, datein, description) VALUES (?,?,?)'; // Prepare the statement $stmt = mysqli_prepare($con, $q); // Asign the values to variables $clientid = $_POST['clientid']; $datein = $_POST['datein']; $description = $_POST['description']; // Bind the variables mysqli_stmt_bind_param($stmt, 'iss', $clientid, $datein, $description); // Execute mysqli_stmt_execute($stmt); // Close the statement mysqli_stmt_close($stmt); if ($q) { // if $r ran with no errors echo "<h3>Thank you!</h3>"; echo "<p>The job has been successfully recorded.</p>"; } else { // if $r ran with errors echo "<h3>Oh, bother...</h3>"; echo "<p>Something goofed. Sorry about that.</p>"; // Debugging message echo "<p>".mysqli_error($con)."</p>"; echo "<p>Query: ".$q."</p>"; } // end of $r IF } else { // Report the errors echo "<div class='error'>"; echo "<h3>Error!</h3> <p>The following error(s) occurred:<br>"; foreach ($errors as $msg) { // Print each error echo " - $msg<br> "; } echo "</p><p>Please try again.</p>"; echo "</div>"; // close error div } // end of (empty($errors)) IF} // end of the main Submit conditional?><!-- FORM!! --> <form action="formJobs.php" method="post"> <p> <label>Client:</label> <?php // prepare statement if ($stmt = mysqli_prepare($con, "SELECT * FROM clients")) { mysqli_stmt_execute($stmt); // bind variables to prepared statement // list all* columns in order of tables selected! mysqli_stmt_bind_result($stmt, $id, $name); // fetch values echo "<select name='clientid'>"; while (mysqli_stmt_fetch($stmt)) { echo "<option value='".$id."'>".$name."</option>"; } echo "</select>"; } ?> <br> <label>Date Received:</label> <input type="date" name="datein" class="focus-glow datepicker job--dateinInput" size="23"> <span class="job--dateinHint">Date placed</span><br> <label>Description:</label> <textarea type="text" maxlength="600" name="description" class="focus-glow" size="28"></textarea> <input type="submit" label="Submit"> </form> <p class="viewResults"><a href="resultsJobs.php">View results</a></p> </div> <!-- / panel --></div> <!-- / columns --></div> <!-- / row --></body></html> Thanks! Edited March 29, 2014 by Steven Link to comment Share on other sites More sharing options...
Ingolme Posted March 29, 2014 Share Posted March 29, 2014 As long as you're using prepared statements properly you won't have problems with SQL injection. There are other things to have in mind, like HTML injection. A user could mess up your site if you let them use HTML in the inputs. Link to comment Share on other sites More sharing options...
Steven Posted March 29, 2014 Author Share Posted March 29, 2014 So, let's say I want to use strip_tags(). My big question is where do I put it? Would I do something like this? // Check for a description entry if (empty($_POST['description'])) { $errors[] = 'You forgot to enter a description'; } else { $description = strip_tags($_POST['description']); } Link to comment Share on other sites More sharing options...
Ingolme Posted March 29, 2014 Share Posted March 29, 2014 Against HTML injection I use htmlspecialchars(). I advise doing it when retrieving the data from the database rather than when putting it in. Link to comment Share on other sites More sharing options...
Steven Posted March 29, 2014 Author Share Posted March 29, 2014 So on my "viewjob.php" page, you would do something like this: echo '<div class="row">'; echo '<div class="large-3 medium-3 columns">'; echo '<div class="panel jobText--panel">'; echo '<p><strong>Client</strong><br>'; echo '<a href="viewclient.php?clientid='.$id.'">'.$name.'</a></p>'; echo '<p><strong>Job Date</strong><br>'; echo $dates.'</p>'; echo '</div>'; // close panel echo '</div>'; // close large-3 column echo '<div class="large-9 medium-9 columns">'; echo '<div class="jobText">'; echo '<h2>Job '.$jobid.'</h2>'; echo '<h3>Description</h3>'; echo '<p>'.htmlspecialchars($description).'</p>'; echo '<h3>Notes & Conversation</h3>'; echo '</div>'; // close jobText echo '</div>'; // close large-7 column echo '</div>'; // close row But couldn't someone make an argument that it'd be best to not allow unwanted characters inside the database to begin with? I don't have any idea, really, which way is best, I've just read some people who make that argument. Link to comment Share on other sites More sharing options...
Ingolme Posted March 29, 2014 Share Posted March 29, 2014 Are the characters really unwanted? I don't see a problem with somebody putting a < in their description as long as it doesn't interfere with the HTML. Link to comment Share on other sites More sharing options...
davej Posted March 29, 2014 Share Posted March 29, 2014 There is this godawful document that will cause your eyes to roll back in your head: https://www.owasp.org/index.php/PHP_Security_Cheat_Sheet Link to comment Share on other sites More sharing options...
thescientist Posted April 3, 2014 Share Posted April 3, 2014 On my "formJobs.php" page, there are three inputs: "Client", "Date", and "Description". The first two inputs are a dropdown list and a datepicker widget, so the user (I'm assuming?) doesn't really have any option or way to submit any invalid data between those first two inputs. They can only select clients already in the "Clients" table, or pick a valid date from a valid calendar pop-out. So I'm not worried about validating and sanitizing those entries, as they should be valid, in theory, because the user has no ability to tinker with it. I wouldn't count those out just because they are in dropdown or the user can't otherwise manipulate the input. There's nothing to stop a user from making a cURL request (for example) to your PHP script and supply their own values. $ curl -x POST 'http://yourdomain.com/formJobs.php?clientId=xx&datein=xxx&description=xxxx" anyone of those params opens the door for potentially malicious code to be sent to you. Link to comment Share on other sites More sharing options...
justsomeguy Posted April 3, 2014 Share Posted April 3, 2014 You don't even need to use any code, there are several tools you can use to create a request to any URL, any method, with any post data. Here are some extensions for Chrome and Firefox to do that:http://stackoverflow.com/questions/4797534/how-do-i-manually-fire-http-post-requests-with-firefox-or-chrome They can only select clients already in the "Clients" table, or pick a valid date from a valid calendar pop-out. So I'm not worried about validating and sanitizing those entries, as they should be valid, in theory, because the user has no ability to tinker with it.You're not trying to protect yourself from people using your site normally. You're trying to protect yourself from people deliberately and specifically attacking your server. Link to comment Share on other sites More sharing options...
thescientist Posted April 3, 2014 Share Posted April 3, 2014 And yes, as JSG there are often times easy web utils normally used for debugging, but could also be used to attack your site. Link to comment Share on other sites More sharing options...
Steven Posted April 3, 2014 Author Share Posted April 3, 2014 Alright, thanks for explaining that. I'll look into those tools. Link to comment Share on other sites More sharing options...
davej Posted April 3, 2014 Share Posted April 3, 2014 Yeah, you have to use a different mindset for web security. For example you wrote a cookie, so it's a safe value, right? No it isn't. Anything external to the server has or will be tampered with. In fact a variety of tools have been created by various people that greatly empower junior hackers by automating various hacking techniques. Some of these tools were written by security experts "to help illuminate the problem" but perhaps the real intent is to increase the demand for their services? One fairly simple tool that is relevant to this discussion is named "Tamper Data." See... http://netsecurity.about.com/od/hackertools/a/What-Hackers-Do-Not-Want-You-To-Know-About-The-Tamper-Data-Firefox-Add-on.htm Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now