5
\$\begingroup\$

I'm starting with PHP so I can save data to a MySQL database. I read a lot and it seems the "escape" strings is not so safe.

This is my code:

<?php 
session_start();
date_default_timezone_set('America/Argentina');
require_once("config/db.php");
$link = mysqli_connect(DB_HOST, DB_USER, DB_PASS);
mysqli_select_db($link, DB_NAME);
$tildes = $link->query("SET NAMES 'utf8'");

$NombreOferta = mysqli_real_escape_string($link,(strip_tags($_POST['NombreOF'], ENT_QUOTES)));
$CantidadArt = mysqli_real_escape_string($link,(strip_tags($_POST['CantidadArt'], ENT_QUOTES)));
$PrecioOf = mysqli_real_escape_string($link,(strip_tags($_POST['PrecioOf'], ENT_QUOTES)));
$NomComercio = mysqli_real_escape_string($link,(strip_tags($_POST['PrecioOf'], ENT_QUOTES)));
$DirComercio = mysqli_real_escape_string($link,(strip_tags($_POST['PrecioOf'], ENT_QUOTES)));
//$userid = "1";
$fecha =date("Y-m-d");
$hora = date("G:i:s<br>", time());

mysqli_query($link, "INSERT INTO ofertas (nombreoferta, cantidadarticulos, precio,user_id,fecha,hora) VALUES('" . $NombreOferta . "', '" . $CantidadArt . "', '" . $PrecioOf . "', '" . $_SESSION['user_id'] . "', '" . $fecha . "', '" . $hora .  "');");

mysqli_close($link);
//echo "<a href=\"OfertaGrabada.html\" target=\"_blank\">title</a>"
header("Location: OfertaGrabada.html"); 
//echo $fecha;
?>

So, is this ok?

\$\endgroup\$
7
\$\begingroup\$

Security

SQL Injection

It's probably[*] secure, but it's not the right way to do it, see for example here why prepared statements are better than escaping. Sooner or later, you will mess up when escaping.

Prepared statements are not difficult to use, and result in code that is more secure and more readable, there is really no good reason not to use them.

[*] if $_SESSION['user_id'] is user supplied in any way, it's not secure.

XSS

Your call to strip_tags doesn't make any sense:

  • ENT_QUOTES isn't a correct value for the second argument, which actually accepts a list of allowed tags.
  • you should encode user input when echoing it, not when inserting it into the database.
  • strip_tags isn't enough to prevent XSS in all contexts.
  • strip_tags mangles your data.

Misc

  • you are mixing the object oriented style and the procedural style of mysqli, which isn't a good idea.
  • variables should start with a lower-case character.
\$\endgroup\$
1
\$\begingroup\$

@tim altready made very good points.

But, one small thing that you're doing horribly wrong and that wasn't metioned:
You haven't grasped the concept of storing dates in a database.

If you're using MySQL, you can create a timestamp field with default CURRENT_TIMESTAMP.
This way, you don't have to do all that dance with dates.

That means that your insert would look like this:

mysqli_query($link, "INSERT INTO ofertas (nombreoferta, cantidadarticulos, precio,user_id) VALUES('" . $NombreOferta . "', '" . $CantidadArt . "', '" . $PrecioOf . "', '" . $_SESSION['user_id'] . "');");

And, in the furute, if you want to get the values back:

SELECT *, date_format(`timestamp`, '%Y-%m-%d') as `fecha`, date_format(`timestamp`, '%k:%i:%s<br>') as `hora` from `ofertas`

Example code only! Not ready for production!


By doing this change, you fix a bug:

$hora = date("G:i:s<br>", time());

This line will produce 16:01:07<bThu, 21 Dec 2000 16:01:07 +0200> instead of the extected result. According to the documentation, about the date() function:

r - RFC 2822 formatted date - Example: Thu, 21 Dec 2000 16:01:07 +0200

Also, don't forget to change your database timezone!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.