CODEWITHSUNDEEP
phpmysqlsql-injection
TechplexEngineer
TechplexEngineer

Reputation: 1928

quote marks in sql causing problems

I have a simple html input textbox in a very simple form. the information form this form is transmitted to a mysql database with an sql string.

Everything works slick, except when someone types " or '. I don't want to limit the users as to what they can type.

Should I do a find and replace to the string before I run the query against the database?

Is there a simple way?

here's some code:

<?php
session_start();
if (empty($_SESSION['user']) && empty($_REQUEST['form'])) //check this code!!1
{
    exit;
}
if (isset($_REQUEST['Submit']))
{
    //echo "Let's process this form!";
    include "config.php";
    include "mail.php";
    if ($_REQUEST['form'] == "profile")
    {//public profile
        //print_r($_REQUEST);
        //"UPDATE `tims`.`pending_profile` SET `nickname` = 'I Don''t Have One' WHERE `pending_profile`.`id` = 1;";
        $sql = "INSERT INTO `tims`.`pending_profile`"
                . "(`id`, `nickname`, `location`, `role`, `yog`, `interests`, `favMoment`, `gainThisYr`, `futurePlans`, `bio`) \n"
                . "VALUES ('" . $_SESSION['id'] . "', '" . $_REQUEST['nickname'] . "', '" . $_REQUEST['town'] . "', '" . $_REQUEST['role'] . "', '" . $_REQUEST['yog'] . "', '" . $_REQUEST['interests'] . "', '" . $_REQUEST['fav_moment'] . "', '" . $_REQUEST['gain'] . "', '" . $_REQUEST['future'] . "', '" . $_REQUEST['bio'] . "')\n"
                . "ON DUPLICATE KEY UPDATE nickname ='" . $_REQUEST['nickname'] . "', location='" . $_REQUEST['town'] . "', role= '" . $_REQUEST['role'] . "', yog='" . $_REQUEST['yog'] . "', interests='" . $_REQUEST['interests'] . "', favMoment='" . $_REQUEST['fav_moment'] . "', gainThisYr='" . $_REQUEST['gain'] . "', futurePlans='" .       $_REQUEST['future'] . "', bio='" . $_REQUEST['bio'] . "'\n";  
        $qry = mysql_query($sql) or die(mysql_error());

//@todo overlay this
//http://flowplayer.org/tools/overlay/index.html
        //send mail to moderators
        include "vars.php";
        $to = $captMail;
        $prof = implode("\n", $_REQUEST);
        $subject = "Moderation Needed";
        $body = $_SESSION['fullname'] . " Has just changed their public profile.\n" .
                "Please login here to moderate their changes:\n" .
                //"http://team2648.com/OPIS/login.php?page=manage".
                "http://www." . $sysurl . "/login.php?page=manage\n" .
                "Best,\n" .
                "Blake\n\n\n" .
                "Click here to accept the profile bleow\n\n" .
                "http://www." . $sysurl . "/login.php?page=manage&acceptID=".$_SESSION['id']."\n" .
                $prof;
        mailer($to, $subject, $body);
        $to = $mentorMail;
        mailer($to, $subject, $body);

        echo "<link href=\"../css/styling.css\" rel=\"stylesheet\" type=\"text/css\" media=\"screen\" />";
        echo "<div class =\"widget\" style=\"width:350px\">";
        echo "Your changes have been saved, they will not go live until reviewed by a moderator";
        echo "<br>";
        echo "<a href=\"../\">Click here to continue</a>";
        echo "</div>";
    }
    exit;
}
$sql = "SELECT * FROM `pending_profile` WHERE id ='" . $_SESSION['id'] . "'";
$qry = mysql_query($sql) or die(mysql_error());
$row = mysql_fetch_assoc($qry);
?>
<!--<h3>Use this page to manage your profile information</h3>-->
<h4>Public Profile</h4>
<strong>NOTE:</strong> Fields filled with [NONE] will not show on the website.
<br />
<form id="profile" name="profile" method="get" action="lib/preview.php">
    <input type="hidden" value="profile" name="form">
    <input type="hidden" value="<?php echo $_SESSION['id']; ?>" name="id">
    <table>
        <tr>
            <td><label for="myname">Hello, My name is:</label><td>
            <td><input type="text" readonly="readonly" name="myname" value="<?php echo $_SESSION['firstname']; ?>"/><td>
        </tr>
        <tr>
            <td><label for="nickname">But I like to be called:</label><td>
            <td><input type="text" name="nickname" value="<?php echo $row['nickname']; ?>"/><td>
        </tr>
        <tr>
            <td><label for="town">I live in:</label><td>
            <td><input type="text" name="town" value="<?php echo $row['location']; ?>"/><td>
        </tr>
        <tr>
            <td><label for="role">My role on the team is:</label><td>
            <td><input type="text" name="role" value="<?php echo $row['role']; ?>"/><td>
        </tr>
        <tr>
            <td><label for="yog">I will graduate High School in:</label><td>
            <td><input type="text" name="yog" value="<?php echo $row['yog']; ?>"/><td>
        </tr>
        <tr>
            <td><label for="interests">Some of my interests are:</label><td>
            <td><input type="text" name="interests" value="<?php echo $row['interests']; ?>"/><td>
        </tr>
        <tr>
            <td><label for="fav_moment">One of my favorite team moments:</label><td>
            <td><input type="text" name="fav_moment" value="<?php echo $row['favMoment']; ?>"/><td>
        </tr>
        <tr>
            <td><label for="gain">I would like to gain the following this year:</label><td>
            <td><input type="text" name="gain" value="<?php echo $row['gainThisYr']; ?>"/><td>
        </tr>
        <tr>
            <td><label for="future">My future plans include:</label><td>
            <td><input type="text" name="future" value="<?php echo $row['futurePlans']; ?>"/><td>
        </tr>
        <tr>
            <td><label for="bio">My Bio:</label><td>
            <td><textarea name="bio" ><?php echo $row['bio']; ?></textarea><td>
        </tr>
    </table>
    * All fields are required.
<?php
include "disclaimer.php";
// @todo add js validation of all fields filled in
?>
    <br><input type="submit" name="Submit" value=" I Agree, Preview "/>

</form>

Upvotes: 4

Views: 5036

Answers (10)

Theophilus Omoregbee
Theophilus Omoregbee

Reputation: 2503

For future folks this worked for me

string addslashes ( string $str )

(PHP 4, PHP 5, PHP 7) addslashes — Quote string with slashes

Returns a string with backslashes before characters that need to be escaped. These characters are single quote ('), double quote ("), backslash () and NUL (the NULL byte). Read more 

<?php
$str = "Is your name O'Reilly?";

// Outputs: Is your name O\'Reilly?
echo addslashes($str);
?>

Upvotes: 0

Bill Karwin
Bill Karwin

Reputation: 562921

As other people have mentioned, you need to handle literal quote characters in your strings or else you're susceptible to SQL injection attacks.

See my presentation SQL Injection Myths and Fallacies for more examples and solutions.

Using query parameters is an effective defense and it's actually easier to write and makes your code look nicer. I recommend using PDO, because it allows you to use named parameters.

Also consider using PHP's heredoc syntax to make it simpler to write SQL code, without worrying about all the double-quotes and concatenating strings.

$sql = <<<END_SQL
INSERT INTO `tims`.`pending_profile`
(`id`, `nickname`, `location`, `role`, `yog`, `interests`, `favMoment`,
 `gainThisYr`, `futurePlans`, `bio`)
VALUES (:id, :nickname, :town, :role:, :yog, :interests, :fav_moment, 
 :gain, :future, :bio)
ON DUPLICATE KEY UPDATE
  nickname    = :nickname,
  location    = :town,
  role        = :role,
  yog         = :yog,
  interests   = :interests,
  favMoment   = :fav_moment,
  gainThisYr  = :gain,
  futurePlans = :future,
  bio         = :bio
END_SQL;

$stmt = $pdo->prepare($sql);
$success = $stmt->execute(array(
    ':id'         => (int) $_SESSION['id'],
    ':nickname'   => $_POST['nickname'],
    ':town'       => $_POST['town'],
    ':role'       => $_POST['role'],
    ':yog'        => $_POST['yog'],
    ':interests'  => $_POST['interests'],
    ':fav_moment' => $_POST['fav_moment'],
    ':gain'       => $_POST['gain'],
    ':future'     => $_POST['future'],
    ':bio'        => $_POST['bio'],
));

note: I changed $_REQUEST to $_POST because you should never use a GET request that modifies your data. Search engines follow links, so you could find Google crawling your site and inadvertently modifying your database. Always use POST requests when the action is going to store or modify something.

For simple cases you don't even have to use parameters, if you can coerce your variable into something that is safe to use, like a plain integer:

$id = (int) $_SESSION['id'];
$sql = "SELECT * FROM `pending_profile` WHERE id = {$id}";
$stmt = $pdo->query($sql);
$row = $stmt->fetch();

Aside from the SQL injection risk your code is also full of a different type of security risk, called Cross-Site Scripting, or XSS. This happens when you echo user-supplied content to the HTML output, without escaping characters that are special in HTML, like < or &. Attackers can cause mischief by entering their name including javascript code.

The fix for XSS is basically to use htmlentities() on anything you echo, if it might have come from user input or any other non-trusted source (like reading from a database or a file or a web service).

<td><input type="text" name="nickname" value="<?php echo htmlentities($row['nickname']); ?>"/><td>

You really need to study these security flaws if you write code for the web. They are a source of a lot of trouble on the internet. See OWASP or the SANS Top 25 Most Dangerous Software Errors.

Upvotes: 4

Galen
Galen

Reputation: 30180

Your code is filled with sql injection opportunities. Look into PDO with prepared statements. Or atleast mysql_real_escape_string. Do not use your code.

Also...

Don't use $_REQUEST. Use the appropriate global $_POST or $_GET. GET requests should not change anything so you should be using $_POST.

Upvotes: 11

Your Common Sense
Your Common Sense

Reputation: 158009

Using traditional "plain" (not parametrized) queries you have to follow 3 rules

  • all strings (quoted portions of data) must be escaped
  • all numbers must be cast to its type explicitly
  • all identifiers or operators should be chosen from previously defined variants.

you can also use some helper function to assemble your query automatically.

function dbSet($fields,&$source) {
  $set='';
  foreach ($fields as $field) {
    if (isset($source[$field])) {
      $set.="`$field`='".mysql_real_escape_string($source[$field])."', ";
    }
  }
  return substr($set, 0, -2); 
}

but your code is tooo dirty to make a working example out of it.
Here is dummy one

$fields = explode(" ","name surname lastname address zip fax phone");
$query  = "INSERT INTO $table SET ".dbSet($fields, $_POST);

Upvotes: 0

codaddict
codaddict

Reputation: 455440

You must use the function mysql_real_escape_string on all your user inputs before you put them in the database.

For example:

change $_REQUEST['nickname'] to mysql_real_escape_string($_REQUEST['nickname'])

Alternatively use prepared statements.

Upvotes: 5

mrjohn
mrjohn

Reputation: 1161

Everyplace where you use a value that can have quotes use the addslashes($mystringwithquotes) method to add slashes before the quotes.

Make sure to not put this around the complete SQL string as you still want to keep the ones you need.

For example in your code:

    $sql = "INSERT INTO `tims`.`pending_profile`" 
                . "(`id`, `nickname`, `location`, `role`, `yog`, `interests`, `favMoment`, `gainThisYr`, `futurePlans`, `bio`) \n" 
                . "VALUES ('" . $_SESSION['id'] . "', '" . $_REQUEST['nickname'] . "', '" . $_REQUEST['town'] . "', '" . $_REQUEST['role'] . "', '" . $_REQUEST['yog'] . "', '" . $_REQUEST['interests'] . "', '" . $_REQUEST['fav_moment'] . "', '" . $_REQUEST['gain'] . "', '" . $_REQUEST['future'] . "', '" . $_REQUEST['bio'] . "')\n" 
                . "ON DUPLICATE KEY UPDATE nickname ='" . $_REQUEST['nickname'] . "', location='" . $_REQUEST['town'] . "', role= '" . $_REQUEST['role'] . "', yog='" . $_REQUEST['yog'] . "', interests='" . $_REQUEST['interests'] . "', favMoment='" . $_REQUEST['fav_moment'] . "', gainThisYr='" . $_REQUEST['gain'] . "', futurePlans='" .       $_REQUEST['future'] . "', bio='" . $_REQUEST['bio'] . "'\n";   
        $qry = mysql_query($sql) or die(mysql_error());

You want to put it around the values addslashes($_SESSION['id']), addslashes($_SESSION['nickname']), etc. but NOT around 

   $sql = "INSERT INTO `tims`.`pending_profile`"

Upvotes: 1

good_evening
good_evening

Reputation: 21759

You can also use (int) if $_POST variable is has to be number.

Upvotes: 2

ngroot
ngroot

Reputation: 1176

Always use parameterized queries when accessing a database. Constructing dynamic SQL statements as you're doing is an invitation to SQL injection attacks (as well as the errors that you're seeing.)

In PHP, the code to do this looks like 

$mysqli = new mysqli('localhost', 'my_user', 'my_password', 'world');
$stmt = $mysqli->prepare("INSERT INTO CountryLanguage VALUES (?, ?, ?, ?)");
$stmt->bind_param('sssd', $code, $language, $official, $percent);

(shamelessly taken from the PHP manual).

Upvotes: 5

Don
Don

Reputation: 1570

You're going to get beat up for not sanitizing the variables from your form before inserting them into your DB.

The PHP Manual has examples for filtering/sanitizing here....

http://php.net/manual/en/filter.filters.sanitize.php

Another popular approach since you're using php and mysql is something like this...

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

More info here:

http://php.net/manual/en/function.mysql-real-escape-string.php

Hope that helps.

Upvotes: 3

Alex Pliutau
Alex Pliutau

Reputation: 21947

Try to use mysql_real_escape_string();

Upvotes: 3

Related Questions