Common Mistakes (AKA How to Not Get Your Shit Fucked Up)

[BrettJay]

[QUOTE=TerabyteS;26997551]Gotta love PHP injection. I wish I could access as admin on the prime minister's website and put porn all over the main page.[/QUOTE]

This isn't a particularly good reason to revive an old thread.

[QUOTE=Fizzadar;26998588]Here's a nice article on security tokens with PHP: [url]http://phpsec.org/projects/guide/2.html[/url][/QUOTE]

But this is.

[Aurorion]

Guys, i have a question, i'm using pdo commands for mysql request, and i need to know if this [B]entire[/B] code is [U][B]safe[/B][/U] :
Sorry, comments are in french and most of the stuff.

<?php
// Les vérifications des variables
//----------------------------------\\
// Si $pseudo n'est pas vide etc
if (! empty ( $pseudo ) and ! empty ( $email ) and ! empty ( $_POST ['mdp'] ) and ! empty ( $_POST ['mdp2'] )) {
	if (preg_match ( "#^[a-zA-Z0-9\[\]_-]{2,14}$#", $_POST ['pseudo'] )) // Verification, le passe doit être composé de 4 à 14 caractères, de a-Z et 0-9 et ne dois pas avoir d'espace
{
		if (preg_match ( '#^[a-zA-Z0-9]{4,18}$#', $_POST ['mdp'] )) // Verification, le passe doit être composé de 4 à 18 caractères, de a-Z et 0-9
{
			if (preg_match ( '#^[a-zA-Z0-9._-]+@[a-zA-Z0-9._-]{2,}\.[a-zA-Z]{2,4}$#', $email )) // Si l'email est correct
{
				if ($_POST ['mdp'] == $_POST ['mdp2']) {
					if (isset ( $_POST ['charte'] )) {
						// Verification que la date de naissance est valide
						if (! ctype_digit ( $annee )) {
							avert ( 'La date de naissance est incorrect.' );
							include_once ('noyau/pied.php');
							exit ();
						}
						if (! ctype_digit ( $mois )) {
							avert ( 'La date de naissance est incorrect.' );
							include_once ('noyau/pied.php');
							exit ();
						}
						if (! ctype_digit ( $jour )) {
							avert ( 'La date de naissance est incorrect.' );
							include_once ('noyau/pied.php');
							exit ();
						}
						if ($annee > 2005 or $annee < 1950) {
							avert ( 'La date de naissance est incorrect.' );
							include_once ('noyau/pied.php');
							exit ();
						}
						if ($mois < 1 or $mois > 12) {
							avert ( 'La date de naissance est incorrect.' );
							include_once ('noyau/pied.php');
							exit ();
						}
						if ($jour < 1 or $jour > 31) {
							avert ( 'La date de naissance est incorrect.' );
							include_once ('noyau/pied.php');
							exit ();
						}
						
						//if (isset ( $_POST ['verif_code'] ) and ! Empty ( $_POST ['verif_code'] )) // Le champ du code de confirmation a été remplis
						//{
						//if ($_POST ['verif_code'] == $_SESSION ['aleat_nbr']) // Si le champ est égal au code généré par l'image
						//{			 
						

						//----------------------------------\\	
						

						// Execution de l'insertion dans la base de donnée.
						$data = $bdd->prepare ( 'INSERT INTO membre VALUES("", 
											"' . $pseudo . '", 
											"' . $email . '", 
											"' . $steam . '", 
											"' . $msn . '", 
											"' . $description . '", 
											"' . $age . '", 
											"' . $date_inscription . '",
											"' . $avatar . '",
											"' . $mdp . '", 
											"membre",
											"' . $ip . '") ' );
						$data->execute ();
					
	//---------------------------------------------------\\
					//}else{avert('Le code de vérification est incorrect.'); include_once('noyau/pied.php'); exit;}
					//}else{avert('Vous n\'avez pas correctement entré le code de vérification.'); include_once('noyau/pied.php'); exit;}
					} else {
						avert ( 'Vous devez avoir lu et approuvé la charte des forums.' );
						include_once ('noyau/pied.php');
						exit ();
					}
				} else {
					avert ( 'Vous n\'avez pas correctement tapper deux fois votre mot de passe.' );
					include_once ('noyau/pied.php');
					exit ();
				}
			} else {
				avert ( 'L\'adresse E-mail entré n\'est pas correct.' );
				include_once ('noyau/pied.php');
				exit ();
			}
		} else {
			avert ( 'Votre mot de passe entré n\'est pas correct ou n\'est pas composé de 4 à 18 caractères' );
			include_once ('noyau/pied.php');
			exit ();
		}
	} else {
		avert ( 'Votre mot de passe entré n\'est pas correct ou n\'est pas composé de 4 à 14 caractères' );
		include_once ('noyau/pied.php');
		exit ();
	}
} else {
	avert ( 'Vous n\'avez pas entrer de pseudo, email ou mot de passe.' );
	include_once ('noyau/pied.php');
	exit ();
}

avert ( 'Vos identifiants ont bien été enregistrés !' );
include_once ('noyau/pied.php');
exit ();

Thanks for the reply.

[StinkyJoe]

[QUOTE=Aurorion;32320914]Guys, i have a question, i'm using pdo commands for mysql request, and i need to know if this [B]entire[/B] code is [U][B]safe[/B][/U]
Thanks for the reply.[/QUOTE]

It's not.

 // Execution de l'insertion dans la base de donnée.
                        $data = $bdd->prepare ( 'INSERT INTO membre VALUES("",
                                            "' . $pseudo . '",
                                            "' . $email . '",
                                            "' . $steam . '",
                                            "' . $msn . '",
                                            "' . $description . '",
                                            "' . $age . '",
                                            "' . $date_inscription . '",
                                            "' . $avatar . '",
                                            "' . $mdp . '",
                                            "membre",
                                            "' . $ip . '") ' );
                        $data->execute ();

You are concatenating user input directly into the SQL string - that is insecure and incorrect. The following should be done using named parameters, as described in the [url=http://php.net/manual/en/pdo.prepare.php]PHP.net manual entry on PDO::prepare[/url]:

$data = $bdd->prepare(
'INSERT INTO membre VALUES(
        :pseudo,
        :email,
        :steam,
        :msn,
        :description,
        :age,
        :date_inscription,
        :avatar,
        :mdp,
        :ip
    )
');
$data->execute(
    compact(
    'pseudo', 'email', 'steam', 
    'msn', 'description', 'age'
    'date_incription', 'avatar',
    'mdp', 'ip'
    )
);

note: just an example, not guaranteed to be fully working or compatible with your current implementation.

That's just the first thing I noticed, I'll have another look when I have more time, and help you with any questions you might have.

PS: This thread could probably do with some updating, same goes for the current sticky at the top.

[Aurorion]

Thank you.

[bbang]

I hate it when i do this

<?php
echo 'HI!';
$string = 'some text';



$string = $text;

echo $text;

?>

The output

HI!

:suicide:

[Aurorion]

The problem is that you define $string at line 3 THEN redefine it in line 7, so it show the last own that is defined. And also becose $text doesn't exist.

Oh wait, nope, the problem is you use $text that isn't defined, with "$string = $text;" you only defined $string, but not $text.

[editline]16th September 2011[/editline]

Also, StinkyJoe, cant i use "?" too ?
Like that one :

$data2 = $bdd -> prepare ('UPDATE topic SET derniermsg=? WHERE id=?');
				$data2 -> execute(array($date, $_GET['id']));

[Lequinx]

[QUOTE=Aurorion;32325424]cant i use "?" too ?
Like that one :

$data2 = $bdd -> prepare ('UPDATE topic SET derniermsg=? WHERE id=?');
				$data2 -> execute(array($date, $_GET['id']));

[/QUOTE]

Just make sure the variables are in order.

[Aurorion]

Yeah that's ok, thank you.
So, if i have correctly understand it, is this code safe ?

<?php
$data_historique = $bdd -> prepare('INSERT INTO historique_eplock VALUES(:id, :idPseudo, :idTopic, :statut, :ouvert, :date) ');
		$data_historique -> execute(array(
		'id' => '',
		'idPseudo' => $_SESSION ['a'] ['id'],
		'idTopic' => $_GET['delock'],
		'statut' => '10',
		'ouvert' => '0',
		'date' => $date
		));

[Lequinx]

Safe from SQL injection, yes