HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Inserting a record into MySQL with a timestamp

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withintorecordmysqltimestampinserting

Problem

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:

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", 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 "title"
header("Location: OfertaGrabada.html"); 
//echo $fecha;
?>


So, is this ok?

Solution

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.

Context

StackExchange Code Review Q#129563, answer score: 7

Revisions (0)

No revisions yet.