0
 <?php
require_once "dbcred.php";
$dbh = testdb_connect ();

/* Obliterate bad input */
$badpasses = $_POST['regpass'];
$salt = '~`!@#$%^&*()_-+=}]{[\|"><';
$secPass = md5($badpasses.$salt);

$newStudent = $dbh->exec ("INSERT INTO Student (uname, pass, fname, lname, email, currGrade)                VALUES('$_POST[reguser]',$secPass,'$_POST[regfirst]','$_POST[reglast]','$_POST[regemail]','$_POST[regclassrank]')");

echo "Thanks for signing up!";

?>

Why is this not submitting to my mysql database anymore? The code below WAS submitting..

 <?php
require_once "dbcred.php";
$dbh = testdb_connect ();

/* Obliterate bad input */


$newStudent = $dbh->exec ("INSERT INTO Student (uname, pass, fname, lname, email, currGrade)                VALUES('$_POST[reguser]','$_POST[regpass]','$_POST[regfirst]','$_POST[reglast]','$_POST[regemail]','$_POST[regclassrank]')");

echo "Thanks for signing up!";

?>
Jshee
  • 2,620
  • 6
  • 44
  • 60

3 Answers3

4

You need to put quotes around $secpass in the query:

$newStudent = $dbh->exec ("INSERT INTO Student (uname, pass, fname, lname, email, currGrade)                VALUES('$_POST[reguser]','$secPass','$_POST[regfirst]','$_POST[reglast]','$_POST[regemail]','$_POST[regclassrank]')");

Just FYI, there are a lot of other problems with your code here. The biggest ones are that salt should be random. You can store it in the database next to the password but having different random salt for every password massively reduces the use of rainbow tables.

Secondly, and this is a much bigger problem, you need to escape your variables using mysql_real_escape_string() or by converting your database access to use PDO. Otherwise you are opening yourself up to a world of pain in the form of SQL injection attacks.

Endophage
  • 21,038
  • 13
  • 59
  • 90
  • @Chris Haha! Very true. I'd forgotten that otherwise I would have linked it originally. – Endophage Aug 02 '11 at 21:45
  • 1
    @user700070 On a more serious note, you really don't want to be hashing passwords with MD5 if you have a choice. Take a look at [this](http://stackoverflow.com/questions/1581610/help-me-make-my-password-storage-safe). – Chris Hepner Aug 02 '11 at 21:46
  • @Chris as long as you use something like SHA512 and random salt you're good for now (and for a little while yet). MD5 is certainly insecure as are smaller block SHA variations. Blowfish actually only uses blocks of 64 bits with keys up to 448 bits, so in terms of complexity, it is actually easier to brute force afaik compared to SHA512. If anyone knows different I'd be interested to know the details, I love security stuff. – Endophage Aug 02 '11 at 21:49
  • Confusingly, CRYPT_BLOWFISH is actually a hashing algorithm based on Blowfish designed for things like passwords. Ideally, you'd use it for a bcrypt implementation, like in [phpass](http://www.openwall.com/phpass/), as referenced in that answer. The whole point of bcrypt is that it's comparatively *really slow* to compute. SHA2, which while not cryptographically broken like MD5 is, is fast enough that you can hash *hundreds of millions* of passwords a second on consumer-grade hardware. [Here's a little more info on that](http://codahale.com/how-to-safely-store-a-password/). – Chris Hepner Aug 02 '11 at 22:04
  • 1
    @Chris ah, I get it. You just make it take ages to calculate the rainbow tables by using a really slow algorithm. That's just plain evil, and I say that with my white hat on. I like it! – Endophage Aug 02 '11 at 23:44
2

In line

$newStudent = $dbh->exec ("INSERT INTO Student (uname, pass, fname, lname, email, currGrade)                VALUES('$_POST[reguser]',$secPass,'$_POST[regfirst]','$_POST[reglast]','$_POST[regemail]','$_POST[regclassrank]')");

You forgot the quotes around $secPass. Try this code:

$newStudent = $dbh->exec ("INSERT INTO Student (uname, pass, fname, lname, email, currGrade)                VALUES('$_POST[reguser]','$secPass','$_POST[regfirst]','$_POST[reglast]','$_POST[regemail]','$_POST[regclassrank]')");
Dador
  • 5,277
  • 1
  • 19
  • 23
1

You didn't put $secPass in quotes in the query. md5 returns just a string, so it should be put in quotes if you want to store it in a string field.

GolezTrol
  • 114,394
  • 18
  • 182
  • 210