Falla di sicurezza nelle mie pagine protette

Ciao

Ho creato un sistema di login mediante il quale un utente per entrare nel suo pannello di controllo e per modificare suoi articoli deve prima autenticarsi.

La sicurezza sull'accesso alle pagine protette del sito accessibili solo agli utenti registrati sembra funzionare.

Ho riscontrato però un problema sull'invio e la modifica degli articoli (con immagini) dei singoli utenti.

L'utente loggato modifica gli articoli in base al suo id (che ho salvato in sessione permettendo così di spostarsi tra le varie pagine) ad ogni articolo insomma viene associato anche l'ID dell'utente per identificare lo stesso.

Mi sono accorto però che modificando nel browser

L'id dell'articolo ad esempio:

http://localhost/multigallery/manager.php?task=mod1&id=12

 al posto del 12 (l'id del singolo articolo che "appartiene" all'utente loggato)

inserisco 9, sempre nella barra degli indirizzi, si visualizzano i dati di un articolo relativi ad un altro utente che io posso modificare impunemente.

La tabella che contiene i dati dell'articolo, come avete già capito, è unica per tutti e tutti gli articoli sono identificati con l'id utente (quindi ID_articolo, ID_utente, nome, descrizione, ecc ecc per essere chiari).

Come possono evitare questa falla nella sicurezza impedendo a utenti malintenzionati di cancellare e modificare (provando ad inserire ID) articoli non propri?

Grazie

inviato 6 anni fa
frankphp
X 0 X

Devi fare un controllo incrociato.

Nello script di aggiornamento o di modifica delle pagine devi controllare che chi vuole modificare la pagina corrisponda al proprietario dell'articolo.

Non puoi usare la sola sessione. Cioè con la sessione dai la possibilità di entrare nelle pagine di "amministrazione" ma poi devi mettere un ulteriore controllo, che è quello che ti ho appena indicato.

ciao

risposto 6 anni fa
Mario Santagiuliana
X 0 X

Devi fare un controllo incrociato.

Nello script di aggiornamento o di modifica delle pagine devi controllare che chi vuole modificare la pagina corrisponda al proprietario dell'articolo.

Non puoi usare la sola sessione. Cioè con la sessione dai la possibilità di entrare nelle pagine di "amministrazione" ma poi devi mettere un ulteriore controllo, che è quello che ti ho appena indicato.

ciao

devo capire bene qui.

il controllo lo devo inserire direttamente nello script che fa l'upload per le modifiche e quello per la cancellazione?

cioè a questo:

$query = "UPDATE images SET Titolo= '$titolo_mod', Descrizione= '$descrizione_mod' WHERE Id = '$id_user'";

dovrei aggiungere una cosa del tipo

 where ID =  '$id_user AND auth = '$auth' // $auth codice identificativo univoco

visto che al momento della registrazione viene prodotto anche un codice di identificazione univoco, dovrei utilizzarlo

nella query (codice che un altro utente non può conoscere ed evitando che possa andare per tentativi con per L'id)??

Dicevi così o intendevi una modifica che ha a che fare con l'intera pagina?

(infatti vedo già in questo forum che se avvicino il mouse sul bottone modifica vedo -nella barra di stato del browser- che tra i parametri passati in get c'è anche un SESC + un codice criptato credo)

Grazie

risposto 6 anni fa
frankphp
modificato 6 anni fa
X 0 X
(infatti vedo già in questo forum che se avvicino il mouse sul bottone modifica vedo -nella barra di stato del browser- che tra i parametri passati in get c'è anche un SESC + un codice criptato credo)

Non confonderti, lascia stare questa cosa.

Dicevi così o intendevi una modifica che ha a che fare con l'intera pagina?

Che "cambia l'intera pagina". Ho messo appositamente le virgolette.

Quello che devi fare è non far visualizzare le possibilità di modifica di una pagina/articolo nel caso che quest'ultimo non appartiene all'utente che vuole apportare le modifiche. Magari dai un messaggio: "Puoi visualizzare la pagina ma non modificarla". Oppure proprio eviti la cosa e nel caso si tenti di vedere un articolo non appartenente all'utente lo rimandi al suo pannello di controllo principale.

risposto 6 anni fa
Mario Santagiuliana
X 0 X

magari se mi inserisci un po' di codice di esempio la cosa mi è più chiara.

Grazie

risposto 6 anni fa
frankphp
X 0 X

Se fornisci lo script per vedere come integrare il tutto...

risposto 6 anni fa
Mario Santagiuliana
X 0 X
Se fornisci lo script per vedere come integrare il tutto...

in pratica la pagina manager.php  è questa (lista degli articoli dell'utente e funzioni per la cancellazione e modifica degli stessi):

session_start();
include ("devlogin/my_config.php");
Pagina_protetta();

// la funzione (che si trova nella pagina inclusa) sopra controlla che il login ha avuto successo e inserisce alcuni dati in sessione quindi possono accedere alla pagina tutti gli utenti loggati..il contenuto ovviamente cambia in base all'ID


......

function dom_delete($id)
{
   $idute=$_SESSION['iduser'];

    $query="SELECT * FROM images user WHERE id = '$id' AND id_user = '$idute'  LIMIT 1";
      $risultato = mysql_query($query) or die("<img src=\"../images/button_cancel.png\"> <span class=\"Stile7\">Errore durante l'esecuzione della query</span>");
      if($riga = mysql_fetch_array($risultato))
      {
      echo "<br /><br /><table align=\"center\"><tr><td>
            <img src=\"images/up.gif\" align=\"center\">
         <table align=\"center\" width=\"700\" background=\"images/table2.gif\">
      <tr><td><img src=\"images/cancella_utente.jpg\"><br /><br /><br /><br />
      
      <div class=\"Stile7\" align=\"center\">Sei sicuro di voler cancellare l'articolo <b><font color=\"#FF0000\"> ".$riga['Titolo']." </font></b>?<br /><br />
      <a href=\"manager.php?task=delete&id=$id\"><img src=\"images/si.gif\" border=\"0\" alt=\"Sì\"></a>
      <a href=\"manager.php?task=lista\"> <img src=\"images/no.gif\" border=\"0\" alt=\"No\"></a></div><br /><br />
      </td></tr></table>";
      echo "<img src=\"images/down.gif\" align=\"center\"></tr></td></table>";
         
      }
       else
       {
        echo "errore";
      }

   }

function lista(){
   $idute=$_SESSION['iduser'];
    $cartella=$_SESSION['cartella'];
   
$righe_per_pagina = 5;
if (!isset($_REQUEST['pagina'])) $pagina = 1;
else {
    if ($_REQUEST['pagina'] <= 0) $pagina = 1;
    else $pagina = $_REQUEST['pagina'];
   }
   

global $path_imgt;

$path_imgt = $path_imgt.$cartella."/";

$query = "SELECT * FROM images  WHERE id_user = '$idute' ORDER BY Titolo";

$esegui = mysql_query($query)
 or die ("Non riesco ad eseguire la richiesta $esegui");
$totr = mysql_num_rows($esegui);

// arrotonda al numero intero + alto
    $numero_pagine = ceil($totr/$righe_per_pagina);   
// calcola il numero della pagina corrente
    $pagina_corrente= ceil($pagina-1/$righe_per_pagina); 

$query.=" LIMIT ".(($pagina-1)*$righe_per_pagina).", ".$righe_per_pagina;
$querylim = mysql_query($query);
 echo " <div align=center><b>Pagina $pagina_corrente di $numero_pagine</b><br>
 <b>Numero articoli trovati: <font color=\"#FF0000\" size=\"5\"> ".$totr." </font></div></b>";    
    
echo "<center><h3>Lista Immagini Galleria</h3></center>";
echo"<p><div align=\"center\"><TABLE border=1 cellspacing=\"0\">
   <TR bgcolor=\"#F3F3F3\">
      <TD>
          <b>Titolo<b/>
      </TD>
      <TD>
         <b>Descrizione</b>
      </TD>
      <TD>
         <b>ID</b>
      </TD>
       <TD>
         <b>Thumb</b>
      </TD>
       <TD>
         <b>Modifca</b>
      </TD>
      <TD>
      <b>Elimina</b>
      </TD>     
     </TR>";
     
$i = 0;  
while ($result = mysql_fetch_array($querylim)) {
   
$risultdesc=stripslashes($result[Descrizione]);
 $i = (++$i % 2);
echo "<TR class=alternate".$i.">
      <TD>
         $result[Titolo]
      </TD>
      <TD>$risultdesc</TD>
      <TD>
          $result[Id]
      </TD>
      <TD>
           <img src=\"".$path_imgt.$result['thumb']."\" border=\"0\">
      </TD>
      <TD align = \"center\">
          <a href=\"manager.php?task=mod1&id=$result[Id]\">Modifica</a>
          
      </TD>
      <TD align =\"center\">
           <a href=\"manager.php?task=dom_delete&id=$result[Id]\">Canc</a>
           
      </TD></TR>";
   }
 echo "</TABLE></div>";
 
  echo "<center><br>";
 if ($numero_pagine > 1) {
//inizio della condizione per creare l'elenco delle pagine
for ($pag = 1; $pag <= $numero_pagine; $pag++) {
echo "[&nbsp<a href=?";
echo "pagina=".($pag)."&clie=$cli&firstinput=$first&secondinput=$second".
" title=\"Vai a pagina $pag\">".$pag."</a>&nbsp]&nbsp";
 }
} // fine "elenco"

echo "</center>"; 
 }
  
 function mod1($modifica_id){
    
    $idute=$_SESSION['iduser'];
    
$query = "SELECT * FROM images WHERE Id = '$modifica_id' AND id_user = '$idute'  LIMIT 1"; //VEDERE QUI
$esegui = mysql_query($query);

$num_rows = mysql_num_rows($esegui);

if($num_rows==0){
die("Stai tentando di modificare un'articolo inesistente o non di tua proprietà!");
}  

while ($result = mysql_fetch_array($esegui))
{
echo "<FORM  action=\"manager.php\" method=post>
<input type=\"hidden\" name=\"task\" value=\"mod2\">
Titolo<p><INPUT TYPE=\"TEXT\"  name=\"titolo_mod\" value=\"$result[Titolo]\"><p>
Descrizione<p><TEXTAREA  name=\"descrizione_mod\" rows=\"4\" cols=\"25\">$result[Descrizione]</TEXTAREA><p>
<INPUT TYPE=\"HIDDEN\"  name=\"id_user\" value=\"$result[Id]\">
<INPUT TYPE=\"SUBMIT\"></FORM>";
 }
 echo "<br><br><a href=\"manager.php\"><b>Torna alla lista</b></a>";
}

function mod2($id_user,$titolo_mod,$descrizione_mod){
   
   $idute=$_SESSION['iduser'];

 if ((strlen($titolo_mod) > 25) || (strlen($descrizione_mod) > 150))
    {
        echo " Attenzione! Il titolo deve essere massimo di 25 caratteri e la descrizione massimo di 150 caratteri. 
      Riprovare<br><br>"; 
      mod1($id_user);  
    }
    
    else {
    
$query = "UPDATE images SET Titolo= '$titolo_mod', Descrizione= '$descrizione_mod' WHERE Id = '$id_user' AND id_user = '$idute' LIMIT 1";
$esegui = mysql_query($query);

if(@mysql_query($query)){
echo ("Hai modificato Il Titolo adesso è: <b>$titolo_mod</b><br>
      La Descrizione adesso è: <b>$descrizione_mod</b><br>");
      
echo ("<A href=\"manager.php?task=mod1&id=$id_user\">Torna alla pagina modifica dati</A><br>");
echo ("<A href=\"manager.php?task=lista\">Torna alla lista utenti</A><br>");
} else {
echo ("Non hai modificato i dati dell'utente.".mysql_error());
  }
 }
}

function delete($id) {
   
   $idute=$_SESSION['iduser'];
    $cartella=$_SESSION['cartella'];
    
   $query = "SELECT * FROM images WHERE Id = '$id' AND id_user = '$idute' LIMIT 1";
   $esegui = mysql_query($query);
   
  $result = mysql_fetch_array($esegui);  
   
   
   $path_nome_filethumb = 'thumbnails/'.$cartella."/". $result["thumb"];
if (file_exists($path_nome_filethumb)) 
{
unlink($path_nome_filethumb); 
}

  $path_nome_filepho = 'photos/'.$cartella."/". $result["Nome"];
if (file_exists($path_nome_filepho)) 
{
unlink($path_nome_filepho); 
}
     mysql_query("delete from images WHERE Id=$id AND id_user = '$idute' LIMIT 1");
echo "<br><br>
   <div align=\"center\"> 
   <table width=\"476\" height=\"124\" background=\"images/done.gif\">
   <tr><td> 
      <p align=\"center\"> <strong> Articolo cancellato correttamente!</strong></td>
   </tr>
   <tr><td> 
   <p align=\"center\"> 
   <img src=\"images/apply_big.gif\">&nbsp;&nbsp; Attendi il caricamento oppure clicca <a href=\"manager.php\">qui</a></td>
   </tr>
   </table>
   </div>";

rindirizzo("manager.php",3);



}

switch($_REQUEST['task']) {       
    case 'mod1':
        mod1($_REQUEST['id']);
       break;
case 'mod2':
        mod2($_REQUEST['id_user'],$_REQUEST['titolo_mod'],$_REQUEST['descrizione_mod']);
        break;
case 'dom_delete':
     dom_delete($_REQUEST['id']);
      break; 
case 'delete':
     delete($_REQUEST['id']);
      break;
   default:
        lista();      
}
?>

Ho fatto delle modifiche che mi hanno suggerito (non so se sono compatibili con quelle che mi hai detto tu)

che  sembrano efficaci anche se il codice è abbastanza sporco (non sono una cima di programmatore  php :-[ :P)

Prima della modifica ad esempio nella funzione mod1:

$idute=$_SESSION['iduser'];

    

$query = "SELECT * FROM images WHERE Id = '$modifica_id' AND id_user = '$idute'  LIMIT 1"; //VEDERE QUI

$esegui = mysql_query($query);

$num_rows = mysql_num_rows($esegui);

if($num_rows==0){

die("Stai tentando di modificare un articolo inesistente o non di tua proprietà!");

}  

dove ho aggiunto la verifica dell'id_user (contenuto in un campo della tabella degli articoli comunque) e relativo messaggio di errore come puoi vedere ; potevo da qui cambiare l'id dell'articolo (di altri utenti) dal browser,

visualizzare i dati relativi nei vari campi html e con il submit fare le modifiche spudoratamente.

Non so se ti è tutto chiaro. Fammi sapere nel caso.

Tu cosa proponevi invece?

Grazie

risposto 6 anni fa
frankphp
X 0 X
Tu cosa proponevi invece?

Di fare un controllo precedente. Non con la query. Questi generi di controli si fanno prima, non dopo.

Allora se l'utente è il proprietario dell'articolo e si è autenticato lo può modificare/aggiornare, altrimenti nisba. Non si fa niente (lo si può reindirizzare.

Ho dato una letta veloce al codice. Sinceramente io lo scriverei in modo differente. Le varie funzioni le metterei magari anche in un file a parte.

Comunque metti un if iniziale che controlli se l'id dell'articolo richiamato appartiene all'utente che sta facendo richiesta, se non è corretto lo rimandi dove preferisci. Credo che l'if lo puoi mettere all'interno del tuo switch.

risposto 6 anni fa
Mario Santagiuliana
X 0 X
Tu cosa proponevi invece?

Di fare un controllo precedente. Non con la query. Questi generi di controli si fanno prima, non dopo.

Allora se l'utente è il proprietario dell'articolo e si è autenticato lo può modificare/aggiornare, altrimenti nisba. Non si fa niente (lo si può reindirizzare.

Ho dato una letta veloce al codice. Sinceramente io lo scriverei in modo differente. Le varie funzioni le metterei magari anche in un file a parte.

Comunque metti un if iniziale che controlli se l'id dell'articolo richiamato appartiene all'utente che sta facendo richiesta, se non è corretto lo rimandi dove preferisci. Credo che l'if lo puoi mettere all'interno del tuo switch.

si lo so che il codice non è da professionisti (infatti non lo sono)

Mi risulta più facile scriverlo in forma procedurale tenendo tutto sott'occhio. Tanto è vero che non uso le classi (anche perchè non le so utilizzare per bene e per quella volta che scrivo qualche riga di codice le trovo anche superflue per me)

Teoricamente ho capito cosa mi suggerisci ma in pratica: L'IF all'interno dello switch??

( e poi Io l'IF di controllo pensavo di averlo già messo)

Come esattamente. Potresti riportarmela qualche riga di codice in modo da poter ragionare su esso.

Grazie

risposto 6 anni fa
frankphp
X 0 X

Non è una cosa difficile.

Hai questo:

switch($_REQUEST['task']) {       

    case 'mod1':

        mod1($_REQUEST['id']);

       break;

     case 'mod2':

        mod2($_REQUEST['id_user'],$_REQUEST['titolo_mod'],$_REQUEST['descrizione_mod']);

        break;

     case 'dom_delete':

     dom_delete($_REQUEST['id']);

      break;

      case 'delete':

     delete($_REQUEST['id']);

      break;

   default:

        lista();     

}

?>

Giusto? E' la parte di codice che fa le modifiche di un articolo ecc ecc giusto?

Fa una cosa tipo questa, ovviamente devi correggere il tutto in modo che sia sintatticamente corretto e adeguato alle tue variabili:

Ti crei una funzione che controlli che l'articolo sia stato creato o meno dall'utente e ti restituisce vero(se appartiene all'utente) o falso e poi fai:

switch($_REQUEST['task']) {       
    case 'mod1':
        if(creato_da_utente($id_articolo, $id_utente))
            mod1($_REQUEST['id']);
        else
            echo "L'articolo che stai tentando di modificare non ti appartiene."
       break;
     case 'mod2':
        if(creato_da_utente($id_articolo, $id_utente))      
            mod2($_REQUEST['id_user'],$_REQUEST['titolo_mod'],$_REQUEST['descrizione_mod']);
        else
            echo "L'articolo che stai tentando di modificare non ti appartiene."
        break;
     case 'dom_delete':
     dom_delete($_REQUEST['id']);
      break; 
      case 'delete':
     delete($_REQUEST['id']);
      break;
   default:
        lista();      
}

La funzione potrebbe essere logicamente così fatta (non è propriamente corretta, correggila secondo le tue necessità):

function creato_da_utente($id_articolo, $id_utente) {
    $query = "SELECT id_articolo FROM utente WHERE id_articolo=$id_articolo AND id_utente=$id_utente";
    $result = mysql_query($query)
    if($result)
      return true;
    else
       return false;
}
risposto 6 anni fa
Mario Santagiuliana
modificato 6 anni fa
X 0 X

appena ho 5 minuti mi metto sotto. Domani ti faccio sapere

In merito alla funzione sia l'id dell'articolo che l'id dell'utente si trovano nella tabella images, come

puoi vedere sopra nel codice, quindi basta riportare il controllo che facevo già (ti trovi?) in modo più ordinato nello switch.

Id_utente lo prendo dalla sessione e lo passo poi insieme a quello dell'articolo alla funzione.

ma poi sono visibili così come le hai messe le variabili nello switch?

Che dici?

Grazie ancora

risposto 6 anni fa
frankphp
X 0 X

No, come detto, ho dato un codice sporco e scorretto. Ho riportato dei passaggi logici da seguire, tutto qui.

Mi sembra giusto che impari ad usare il php bene per fare queste cose e l'unico sistema è fare "da se" ;)

Ciao

risposto 6 anni fa
Mario Santagiuliana
X 0 X

No, come detto, ho dato un codice sporco e scorretto. Ho riportato dei passaggi logici da seguire, tutto qui.

Mi sembra giusto che impari ad usare il php bene per fare queste cose e l'unico sistema è fare "da se" ;)

Ciao

ok magari le inserisco in sessioni e le richiamo così.

Grazie dell'aiuto e ciao

risposto 6 anni fa
frankphp
X 0 X

oppure per non complicarmi la vita richiamo la funzione all'interno delle altre funzioni di modifica.

risposto 6 anni fa
frankphp
X 0 X

Come preferisci.

A me sembra più logico mettere il bivio in un punto, cioè o è così o se no ti rimando da un'altra parte...

Poi è la stessa cosa.

 :bye:

risposto 6 anni fa
Mario Santagiuliana
X 0 X
Effettua l'accesso o registrati per rispondere a questa domanda