Pergunta

Checking my web site with google webmaster tools I found a strange thing :

somebody tried to use to link to my site using this (I change the real name of my site for obvious security reasons ) :

....://mysite.com/tarifs.php?annee=aaatoseihmt&mois=10&cours=1828

I try it and understand it was a sql injection with the results :

Warning: mktime() expects parameter 6 to be long, string given in /home/..../public_html/..../tarifs.php on line 72

my code line 72 is :

mktime (0, 0, 0, $mois, "01", $annee)

part of this:

<?php
include ("include.php");

if (!$link = mysql_connect($host, $user, $pass)) {
    echo "Could not connect to mysql";
    exit;
}

if (!mysql_select_db($bdd, $link)) {
    echo "Could not select database";
    exit;
}

mysql_query("SET NAMES 'utf8'");

$annee = "";
$mois = "";
$stage = "";

if (isset($_GET['annee'])) {$annee=$_GET['annee'];}
if (isset($_GET['mois'])) {$mois=$_GET['mois'];}
if (isset($_GET['stage'])) {$stage=$_GET['stage'];}

if($annee == "")
{

    $annee = date("Y");
}

if($mois == "")
{

    $mois = date("m");
}


$date_du_jour = date("d")."-".date("m")."-".date("Y");

if($mois == "12")
{
    $mois_precedent = "11";
    $mois_suivant = "01";
    $annee_mois_precedent = $annee;
    $annee_mois_suivant = $annee + 1;
}
elseif($mois == "01")
{
    $mois_precedent = "12";
    $mois_suivant = "02";
    $annee_mois_precedent = $annee - 1;
    $annee_mois_suivant = $annee;
}
else
{
    $mois_precedent = sprintf("%02s", $mois-1);
    $mois_suivant = sprintf("%02s", $mois+1);
    $annee_mois_precedent = $annee;
    $annee_mois_suivant = $annee;
}


$jour_en_cours = date("d");

$mois_francais = array("Janvier", "Février", "Mars", "Avril", "Mai", "Juin", "Juillet", "Août", "Septembre", "Octobre", "Novembre", "Décembre");

$dt_deb_genere = $annee."-".$mois."-01";
$dt_fin_genere = $annee_mois_suivant."-".$mois_suivant."-01";

$dt_date = mktime (0, 0, 0, $mois, "01", $annee);
$jour_de_la_semaine = date("w", $dt_date);
?>

what can I do to protect my site against this ?

I tried to understand how to it with "similar question" but I think I am to new to php and mysql to be able to understand.So any help is really great !

Thanks if you can help on this ! I worked hard for months now on my site and don't want to lose my business.

.blc.


THEN !!! I have made few little change after the code provided by Lucanos (THANKS !!!!) :-) :

<?php
include ("include.php");

if (!$link = mysql_connect($host, $user, $pass)) {
    echo "Could not connect to mysql";
exit;
}

if (!mysql_select_db($bdd, $link)) {
echo "Could not select database";
exit;
}

mysql_query("SET NAMES 'utf8'");

$annee = '';
$mois = '';
$stage = '';

if( isset( $_GET['annee'] ) )
{
$annee = preg_replace( '/\D/' , '' , $_GET['annee'] );
if( !$annee || !( $annee<=2015 && $annee>=2013 ) )
// Allows you to set an expected range for this value
// code here expects a number between 2013 and 2015 inclusive
$annee = '';
}

if( isset( $_GET['mois'] ) )
{
$mois = preg_replace( '/\D/' , '' , $_GET['mois'] );
if( !$mois || !( $mois<=12 && $mois>=1 ) )
// I assume this is the Month, with a range of 1 to 12
$mois = '';
}

if (isset($_GET['stage'])) {$stage=$_GET['stage'];}

if($annee == '')
{
// Récupération de l'année en cours
$annee = date('Y');
}

if($mois == '')
{
// Récupération du mois en cours
$mois = date('m');
}

// Récupération de la date du jour

$date_du_jour = date( 'd-m-Y' );

if($mois == '12')
{
$mois_precedent = '11';
$mois_suivant = '01';
$annee_mois_precedent = $annee;
$annee_mois_suivant = $annee + 1;
}
elseif($mois == '01')
{
$mois_precedent = '12';
$mois_suivant = '02';
$annee_mois_precedent = $annee - 1;
$annee_mois_suivant = $annee;
}
else
{
$mois_precedent = sprintf('%02s', $mois-1);
$mois_suivant = sprintf('%02s', $mois+1);
$annee_mois_precedent = $annee;
$annee_mois_suivant = $annee;
}

// Récupération du jours en cours
$jour_en_cours = date('d');

$mois_francais = array(
'Janvier' , 'Février' , 'Mars' ,
'Avril' , 'Mai' , 'Juin' ,
'Juillet' , 'Août' , 'Septembre' ,
'Octobre' , 'Novembre' , 'Décembre'
);

$dt_deb_genere = "{$annee}-{$mois}-01";
$dt_fin_genere = "{$annee_mois_suivant}-{$mois_suivant}-01";

$dt_date = mktime( 0 , 0 , 0 , $mois*1 , 1 , $annee*1 );
$jour_de_la_semaine = date( 'w' , $dt_date );
?>

tried (with no success until now) to add a condition testing if "stage" exist in data base to avoid a call like stage=200 which do not exist ans display an empty calendar in the page. But in final I miss something here (I didn't include it in the past code )

$sql_stage = "select * from data where type_data = 'STAGE' and ind_valide = 1 and ind_etat = 1 order by sous_titre, id_type_1, ordre";
$result_stage = mysql_query($sql_stage, $link);
$existingstage = '';


while ($row_stage = mysql_fetch_array($result_stage))
{
$existingstage = $row_stage["id_data"];

if( isset( $_GET['stage'] ) )
{
    $stage = preg_replace( '/\D/' , '' , $_GET['stage'] );

    if( !$stage || !( $stage= $existingstage ) )

    $stage = '';
}
}
Foi útil?

Solução

Never Trust User Input

Anytime you use a value from a form, or extracted from a URL, make sure that you test it, sanitise it and/or escape it before you use it. Anywhere.

So, for instance, with your code, I would edit it as follows:

<?php
include ("include.php");

// Might be worth putting this into the "include.php" file, or a function
// to do the same thing. Especially if you connect to the DB regularly.
if (!$link = mysql_connect($host, $user, $pass)) {
    echo "Could not connect to mysql";
    exit;
}

// Same as above...
if (!mysql_select_db($bdd, $link)) {
    echo "Could not select database";
    exit;
}

// And again...
mysql_query("SET NAMES 'utf8'");


$annee = '';
$mois = '';
$stage = '';

if( isset( $_GET['annee'] ) )
{
    $annee = preg_replace( '/\D/' , '' , $_GET['annee'] );
    if( !$annee || !( $annee<=2020 && $annee>=1970 ) )
        // Allows you to set an expected range for this value
        // My code here expects a number between 1970 and 2020 inclusive
        $annee = '';
}
if( isset( $_GET['mois'] ) )
{
    $mois = pre_replace( '/\D/' , '' , $_GET['mois'] );
    if( !$mois || !( $mois<=12 && $mois>=1 ) )
        // I assume this is the Month, with a range of 1 to 12
        $mois = '';
}
if( isset( $_GET['stage'] ) )
{
    $stage = pre_replace( '/\D/' , '' , $_GET['stage'] );
    if( !$stage || !( $stage<=100 && $stage>=0 ) )
        // Again, assuming 1-100
        $stage = '';
}

if( $annee=='' )
    $annee = date( 'Y' );

if( $mois=='' )
    $mois = date( 'n' );

$date_du_jour = date( 'd-m-Y' );

if( $mois=='12' )
{
    $mois_precedent = '11';
    $mois_suivant = '01';
    $annee_mois_precedent = $annee;
    $annee_mois_suivant = $annee + 1;
}
elseif( $mois=='01' )
{
    $mois_precedent = '12';
    $mois_suivant = '02';
    $annee_mois_precedent = $annee - 1;
    $annee_mois_suivant = $annee;
}
else
{
    $mois_precedent = sprintf( '%02s' , $mois-1 );
    $mois_suivant = sprintf( '%02s' , $mois+1 );
    $annee_mois_precedent = $annee;
    $annee_mois_suivant = $annee;
}


$jour_en_cours = date( 'd' );

$mois_francais = array(
    'Janvier' , 'Février' , 'Mars' ,
    'Avril' , 'Mai' , 'Juin' ,
    'Juillet' , 'Août' , 'Septembre' ,
    'Octobre' , 'Novembre' , 'Décembre'
);

$dt_deb_genere = "{$annee}-{$mois}-01";
$dt_fin_genere = "{$annee_mois_suivant}-{$mois_suivant}-01";

$dt_date = mktime( 0 , 0 , 0 , $mois*1 , 1 , $annee*1 );
$jour_de_la_semaine = date( 'w' , $dt_date );

?>

Outras dicas

For the future, use prepared statements. They allow you to send the query first and send the values afterwards which prevents injection. I prefer using PDO.

Anyway, rule number one in databases: validate or sanitize user input.


If you are sure that the input should be a number, just force it to a number:

$number = intval($_GET['number']);

In this case, if the user changes it to a bit of text, intval() will return 0.


For the mysql_ functions, if the input is a string, use mysql_real_ecsape_string():

$string = mysql_real_eascape_string($_GET['string']);

This will escape all "unwanted" characters that could affect your SQL query. However, this function is compromised too.

Use prepared statements and parameterized queries. These are SQL statements that are sent to and parsed by the database server separately from any parameters. This way it is impossible for an attacker to inject malicious SQL.

You basically have two options to achieve this:

  1. Using PDO
  2. Using mysqli

Required :

  • Protect your database layer using more protective functions such as mysqli
  • Escape all of your statements
  • Validate all of your inputs

Optional/lazy :

  • Get rid of the $_GET variables and either use $_POST or $_SESSION

Try:

if($annee == "" || intval($annee) == 0 ) {
    $annee = date("Y");
}
Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top