2

I want to update a MySQL database with an input form, essentially a settings page where a user can update/change their information. With this code below, the echo at the bottom runs and I'm not getting any errors, but the database isn't updated. I tested it both on my local machine and on a server and it didn't work with either, although I'm not sure that would have anything to do with it. $_SESSION['yourName'] is already set when the user logs in, so it will equal the current value in the database. I know there are tons of sql injection hole in here, but this is just a proof-of-concept code, not ready to go live. I tried running the query in phpMyAdmin and it worked fine.

PHP

if(isset($_POST['name']) || isset($_POST['password']) || isset($_POST['location']) || isset($_POST['img'])) {
 //put this in an external file for a little extra security
 $username = "my_username";
 $password = "my_password";
 $database = "database_name";
 $con = mysql_connect('localhost',$username,$password);
 if (!$con) {
    die('Oh poop.... Could not connect: ' . mysql_error());
 }
 //print item from database
 mysql_select_db($database, $con);

 if(isset($_POST['name'])) {
    $query = 'UPDATE users SET username=' . $_POST['name'] . 'WHERE username=' . $_SESSION['yourName'];
    mysql_query($query);
    $_SESSION['yourName'] = $_POST['name'];
 }
 if(isset($_POST['password'])) {
    $query = 'UPDATE users SET password=' . $_POST['password'] . 'WHERE username=' . $_SESSION['yourName'];
    mysql_query($query);
 }
 if(isset($_POST['location'])) {
    $query = 'UPDATE users SET location=' . $_POST['location'] . 'WHERE username=' . $_SESSION['yourName'];
    mysql_query($query);
    $_SESSION['location'] = $_POST['location'];
 }
 if(isset($_POST['img'])) {
    $query = 'UPDATE users SET img=' . $_POST['img'] . 'WHERE username=' . $_SESSION['yourName'];
    mysql_query($query);
    $_SESSION['img'] = $_POST['img'];
 }
 echo '<script type="text/javascript">alert("changes saved")</script>';
}

HTML

<form method="post">
    new name: <input type="text" name="name" /><br />
    new password: <input type="text" name="password" /><br />
    new location: <input type="text" name="location" /><br />
    new img: <input type="text" name="img" /><br />
    <input type='submit' name='save' value='save' />
    <input type='submit' name='logout' value='logout' />
</form>
John Woo
  • 258,903
  • 69
  • 498
  • 492
Hat
  • 1,691
  • 6
  • 28
  • 44
  • `var rq = new XMLHttpRequest(); rq.open('POST', 'update.php', false); rq.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded'); rq.send('password=' + encodeURIComponent("'password';--"));` Yay, I can log into any account! Please learn about SQL injection prevention and stop putting unescaped user input directly into your queries. Prepared statements are nice, especially given that PHP 5.5 deprecates the `mysql_` extensions. – Ry- Dec 12 '12 at 01:21
  • ... but the problem you're seeing is because the quotes around the strings are missing. – Ry- Dec 12 '12 at 01:22

2 Answers2

7

try to modify you php statement by using double quotes instead of single quotes.

$query = "UPDATE users SET username='" . $_POST['name'] . "' WHERE username= '" . $_SESSION['yourName'] . "'";

but the query above is vulnerable with SQL Injection, please read the article below,

Community
  • 1
  • 1
John Woo
  • 258,903
  • 69
  • 498
  • 492
  • that'd do it thanks! as said in the question this is just some test code and nowhere near ready to go live. but its always good to know people will point out the sql holes – Hat Dec 12 '12 at 01:45
3

I recommend using PDO. The php mysql object is discouraged in favor of PDO or mysqli.

PDO: http://php.net/manual/en/book.pdo.php

Also you will want to update your rows on a primary key column (id or user_id) for your WHERE statement or you may allow multiple user rows to be effected.

Depending on your application, I would advise not storing passwords in your database. Store a salted hash of the password they enter.

Password Hashing: http://php.net/manual/en/faq.passwords.php

if(isset($_POST['name']) || isset($_POST['password']) || isset($_POST['location']) || isset($_POST['img'])){
//put this in an external file for a little extra security  
$username = "my_username";
$password = "my_password";
$database = "database_name";                                
try{
    $conn=new PDO('mysql:dbname='.$database.';host=localhost;port=3306',$username,$password);
}
catch(PDOException $ex){
    die('Could not connect: '.$ex->getMessage());
}
$stm = $conn->prepare("UPDATE users SET username=?,password=?,location=?,img=? WHERE user_id=?");
$stm->execute(array($_POST['name'],$_POST['password'],$_POST['location'],$_POST['img'],$_SESSION['user_id']));
}   
Clayton Bell
  • 424
  • 4
  • 4