-2

I'm attempting to setup a query through php to a MySQL database. Within the query string I have placed functions and thus have used the dot (.) operator with string closures as seen below. The issue is that my query is not going through and try as i might I can't seem to make out the error. Thanks for any help in advance. :)

$query = "INSERT INTO `foo` (`ip`, `time`, `date`, `reason`) VALUES ('".strval(getUserIpAddr())."', '".$time."', '".$date."', '".$reason."')";
Cui Bono
  • 13
  • 6
  • 1
  • 2
    Have you output the resulting string to confirm that it looks valid? Are you actually _executing_ the query somewhere? – Patrick Q Mar 27 '19 at 21:26
  • 1
    **Warning:** You are wide open to [SQL Injections](http://php.net/manual/en/security.database.sql-injection.php) and should really use parameterized **prepared statements** instead of manually building your queries. They are provided by [PDO](http://php.net/manual/en/pdo.prepared-statements.php) or by [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Never trust any kind of input, especially that which comes from the client side. Even when your queries are executed only by trusted users, [you are still in risk of corrupting your data](http://bobby-tables.com/). – Dharman Mar 27 '19 at 21:26
  • See also https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php – Blackhole Mar 27 '19 at 21:26
  • *"and thus have used the dot (.) operator"*. No! You should never concatenate data into your SQL statements like this. Use prepared statements. – Dharman Mar 27 '19 at 21:27
  • What the others haven't specifically said (and to explain a bit better) is if `$reason= "it's a bad idea not to prepare queries"` (just for example) because you are not preparing you query this `it's` or specifically the quote `'` will wreck havoc on your SQL `INSERT INTO foo (reason) VALUES ('it's a bad idea not to prepare queries')` You see how the `'` completes the other quote in your SQL `INSERT INTO foo (reason) VALUES ('it'`, now you have a syntax error because `s a bad idea not to prepare queries'` is just chilling there. Much worse can happen than this, this is just a brief example. – ArtisticPhoenix Mar 27 '19 at 21:31
  • FYI - `my string closures are wrong somewhere` - if you mean the quotes `'` you don't use them in prepared statements your query would take this form `"INSERT INTO foo (ip,time,date,reason)VALUES(?,?,?,?)"` - then the data is sent as a separate call. This bypasses any quoting issues. ***PS***. the **backtic** is what you use for `code` block, in comments here. So I removed them, but you should keep them for things like `date` which is a reserved word in MySQL (and many DBs), they just don't play nice with them in the comment. – ArtisticPhoenix Mar 27 '19 at 21:39

1 Answers1

0

As you should already be aware, your code has security issues so not going to get into that. I don't see any error handling in your code, so can only assume that is why you are not seeing an error. In order to use PDO, you need the driver loaded on the server so keep that in mind. I will reiterate that you should be using prepared statements, here's an example.

$dsn = 'mysql:host=localhost;dbname=testdb';
$username = 'username';
$password = 'password';
$options = array(
    // options that apply to your configuration
); 

try {
    $db = new PDO($dsn, $username, $password, $options);

    $sql = "INSERT INTO foo (`ip`, `time`, `date`, `reason`) 
            VALUES (:ip, :time, :date, :reason)";

    $stmt = $db->prepare($sql);
    $stmt->bindValue(':ip', strval(getUserIpAddr()), PDO::PARAM_STR);
    $stmt->bindValue(':time', $time, PDO::PARAM_STR);
    $stmt->bindValue(':date', $date, PDO::PARAM_STR);
    $stmt->bindValue(':reason', $reason, PDO::PARAM_STR);

    $stmt->execute();
} catch (PDOException $e) {
    echo "Exception: " . $e->getMessage();
}
EternalHour
  • 8,308
  • 6
  • 38
  • 57
  • Thank you very much! I appreciate everyone's help. I'll make sure to get these changes incorporated. As for my original issue, found out why. I did not auto increment my primary entry and it thus threw a duplicate entry error on my id column. :) – Cui Bono Mar 28 '19 at 17:16