Questo script è vulnerabile alla mysql injection?

[giggioman00]

Sapreste dirmi se è vulnerabile?

Codice: [Seleziona]

<?php

if (  $_POST['option'] == "com_content"
      && $_POST['view'] == "video"
      && is_numeric($_POST['id']))
{
      // connect to the database
      include_once("../configuration.php");
      $cg = new JConfig;
      $con = mysqli_connect($cg->host,$cg->user,$cg->password,$cg->db);
      if (mysqli_connect_errno()){
        die('n/a');
    }
    
$idpos = intval(mysql_real_escape_string($_POST['id']));
    
    // grab the new hit count
    $query = "SELECT  times_viewed
                  FROM  ".$cg->dbprefix."hdflv_upload
                  WHERE   `id` = " . $idpos . "
                  LIMIT 1;
     ";    
     
     $new_hits = mysqli_fetch_assoc(mysqli_query($con,$query));
     
     
    // add new hit
    $addone = $new_hits['times_viewed']+1;
    $queryadd = "UPDATE ".$cg->dbprefix."hdflv_upload
                SET times_viewed=".$addone."
                  WHERE   `id` = " . $idpos . ";
     ";    
    mysqli_query($con,$queryadd);

      // close the connection to the database
      mysqli_close($con);
    
      echo $addone;
    
}

?>

[giovi]

Se sei all'interno di joomla perché utilizzi mysqli_connect? C'è una classe molto più comoda per fare query sul db di joomla che è JDatabase:


https://docs.joomla.org/Selecting_data_using_JDatabase
https://docs.joomla.org/Inserting,_Updating_and_Removing_data_using_JDatabase


Inoltre per  ottenere valori dalle superglobali post, get, cookie, session, ecc... sarebbe opportuno utilizzare i filtri messi a disposizione dalla classe JInput: https://docs.joomla.org/Retrieving_request_data_using_JInput

Così com'è il codice è soggetto ad sql injection perché non utilizzi alcun filtro

[giggioman00]

Scusa, quello è il codice iniziale, ora è cambiato molto. Comunque più che altro mi interessa sapere se rischio di venire hackerato via mysql injection usando questo script:

Codice: [Seleziona]

<?php

// Define our querystring variables
$option = (array_key_exists('option', $_POST) ? $_POST['option'] : false);
$view   = (array_key_exists('view', $_POST) ?  $_POST['view'] : false);
$id     = (array_key_exists('id', $_POST) ?  intval($_POST['id']) : false);

// Check wheter our variables are correct in order to proceed
if ($option == 'com_content' && $view == 'video' && $id > 0) {

    // Connect to the database
    include_once("../configuration.php");
    $cg = new JConfig;
    $con = mysqli_connect($cg->host,$cg->user,$cg->password,$cg->db);
    if (mysqli_connect_errno()) {
        die('n/a');
    }

    // Add new hit: Update the `times_viewed` field corresponding to specific video id
    $query = "UPDATE " . $cg->dbprefix . "hdflv_upload 
                        SET `times_viewed` = `times_viewed` + 1 
                        WHERE `id` LIKE " . $id . ";";

    if (mysqli_query($con, $query) === true) {

    // Get the updated `times_viewed` of video id
        $query  = "SELECT `times_viewed` 
                             FROM " . $cg->dbprefix . "hdflv_upload 
                             WHERE `id` LIKE " . $id . ";";
        $result = mysqli_fetch_assoc(mysqli_query($con, $query));
    }

    // close the connection to the database
    mysqli_close($con);
    $addone = $result['times_viewed'];
    echo $addone;
}
?>


Comunque sono su joomla ma al tempo stesso non lo sono... è difficile da spiegare...

[giovi]

vale quanto detto nel post precedente: a livello di sicurezza non è cambiata una virgola. se non filtri ciò che ti arriva da input non sei al sicuro dalle sql inc

[giggioman00]

PEr ciò che arriva da input ti riferisci allo script che richiama questo codice?

[giovi]

Se in post ti mando un id="5; DROP TABLE ...... " ti cancello tutto...
Con una sql injection la tua query diventerà:

Codice: [Seleziona]

"UPDATE  .... hdflv_upload  ... SET `times_viewed` = `times_viewed` + 1 WHERE `id` LIKE 5; DROP TABLE ..... ;
Se ti aspetti un id devi filtrare per int

[mmleoni]

arrivo un po' in ritardo ma...

il numero è filtrato : intval($_POST['id'])
come vecchio programmatore C io preferisco lo stile con cast esplicito, piuttosto che l'uso di una funzione, quindi meglio (int)$_POST['id'].

lasciati però dire che il tuo codice lascia un poco perplessi:

array_key_exists('option', $_POST)
chi ha detto che l'array esista in quel contesto? concettualmente è meglio empty()

$option = (array_key_exists('option', $_POST) ? $_POST['option'] : false);
va bene che php non è granché tipizzato, ma perché mischiare stringhe e boolean?
visto anche che poi aspetti un valore stringa
if ($option == 'com_content' &&
questo è il modo migliore di fare casini... 


include_once("../configuration.php");
secondo me qui ci va un require_once


WHERE `id` LIKE " . $id
LIKE serve a comparare le stringhe, non i numeri. il match deve essere stretto, quindi ci va un '='


in conclusione, scusami, non so che stai facendo, ma so che lo stai facendo nel modo sbagliato.
(fermi restando i discorsi sull'essere fuori dal framework)

ciao,
marco