SQL vulnerable

Door Jeroen dj op 01-09-2015 20:59 gewijzigd op 01-09-2015 22:55

2.354 views

Hallo Iedereen!

Mijn redirect bestand is vulnerable voor GET.
Het 1e bestand met een id 'bestand.php?id=184' word geredirect naar redirect.php via een button:

<form class="LoginForm" action="redirect1.php?id=<?php echo $_GET['id']; ?>" method="post" autocomplete="off">


<?php
error_reporting(0);
$servername = "localhost";
$username = "test_usern";
$password = "test_passwd";
$dbname = "test_db";
 
date_default_timezone_set("Europe/Amsterdam");
 
$ip = 'unknown';
if(!empty($_SERVER['HTTP_X_FORWARDED_FOR']))
{
   $ip = $_SERVER['HTTP_X_FORWARDED_FOR'];
}
elseif(!empty($_SERVER['REMOTE_ADDR']))
{
   $ip = $_SERVER['REMOTE_ADDR'];
}
else
{
   user_error("Uh-oh! Neither IP variable was set.");
}  
 
 
$fname = $_REQUEST['fname'];
$fname1 = $_REQUEST['fname1'];
$fname2 = $ip;
$fname3 = $_GET['id'];
$fname4 = date('Y/m/d H:i:s');
try {
    $conn = new PDO("mysql:host=$servername;dbname=$dbname", $username, $password);
    // set the PDO error mode to exception
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $sql = "INSERT INTO logs (test, test1, test2, test3, test4)
    VALUES ('$fname', '$fname1', '$fname2', '$fname3', '$fname4')";
    // use exec() because no results are returned
    $conn->exec($sql);
    header("Location: index.php");
    }
catch(PDOException $e)
    {
    header("Location: index.php");
    }
$conn = null;
?>

Via het programma: SQLMap kwam ik erachter dat mijn form hier vulnerable voor is.
Hiermee kon ik zo'n beetje mijn hele database uitlezen en dit is niet mijn bedoeling!
Ik ben zelf niet een held in SQL maar ziet iemand een oplossing? :)

Ariën

01-09-2015 21:14 gewijzigd op 01-09-2015 21:14

Kan je de code in het topic plaatsen. Straks is je pastebin verdwenen, en dan is dit topic nutteloos.

Er zijn twee opties:

Handmatig escapen:
$pdo->quote($_GET['text']);

Of prepared statements:
http://www.phptuts.nl/view/27/5/

De laatste kan ik aanbevelen.

Johan K

01-09-2015 21:25

Als ik de code zo zie, neem ik aan dat "$_GET['id']" gelijk is aan de "UNIQUE ID / Auto increment" in jouw database. Het maakt niet uit of je PDO gebruikt, of een integer escaped een database id mag gewoon niet gebruikt worden in een post, get, header of cookie request aangezien een getal gewoon niet te verifiëren valt.

Je kan een SQL injection tegen gaan door deze inderdaad te laten escapen, maar dat houd niet weg dat ik het getal ID in elk ander getal kan veranderen zodat ik misschien de password van gebruiker "id" kan veranderen vanuit mijn sessie.

Thomas van den Heuvel

01-09-2015 21:51

Of gewoon je input te filteren, m.a.w. controleer of $_GET['id'] een auto-increment id betreft.

Tevens, je gebruikt al PDO, maak er dan ook op een juiste wijze gebruik van, maak gebruik van (desnoods ge-emuleerde) prepared statements.

$pdo->quote() lijkt mij ~~niet~~ nooit de juiste werkwijze eigenlijk.

Jeroen dj

01-09-2015 21:59 gewijzigd op 01-09-2015 22:59

Ik ga proberen een prepared statement te maken, mijn sql kennis is niet al te hoog, ga het wel proberen.

als toch iemand anders de code voor me zou kunnen maken zou helemaal top zijn, wil het namelijk zo snel mogelijk oplossen

[size=xsmall]Toevoeging op 01/09/2015 22:58:29:[/size]


<?php
error_reporting(0); 

date_default_timezone_set("Europe/Amsterdam");

$ip = 'unknown'; 
if(!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) 
{ 
   $ip = $_SERVER['HTTP_X_FORWARDED_FOR']; 
} 
elseif(!empty($_SERVER['REMOTE_ADDR'])) 
{ 
   $ip = $_SERVER['REMOTE_ADDR']; 
} 
else 
{ 
   user_error("Uh-oh! Neither IP variable was set."); 
}  

try 
{ 
	$fname = $_REQUEST['fname'];
	$fname1 = $_REQUEST['fname1']; 
	$fname2 = $ip;
	$fname3 = $_GET['id'];
	$fname4 = date('Y/m/d H:i:s');
	
    $db = new PDO('mysql:host=localhost;dbname=test_db','test_usern','test_passwd'); 
    $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); 
     
    $sql = " 
        INSERT INTO auth (test, test1, test2, test3, test4) 
        VALUES (:fname,:fname1,:fname2,:fname3,:fname4) 
        "; 
     
    $stmt = $db->prepare($sql); 
	$stmt->bindParam(':test', $fname, PDO::PARAM_STR);
	$stmt->bindParam(':test1', $fname1, PDO::PARAM_STR);
	$stmt->bindParam(':test2', $fname2, PDO::PARAM_STR);
	$stmt->bindParam(':test3', $fname3, PDO::PARAM_STR);
	$stmt->bindParam(':test4', $fname4, PDO::PARAM_STR);
    $stmt->execute(); 
} 
catch(PDOException $e)
{
	header('Location: index.php');
} 
?>

Heb zelf wat geprobeert te maken maar krijg een foutmelding:
Error: SQLSTATE[HY093]: Invalid parameter number: parameter was not defined
Ik wil dit zo snel mogelijk oplossen want dat is een groot lek in mijn site!
En als de code hierboven zou werken, zou dit dan wel veilig zijn?

Ivo P

02-09-2015 09:07

er staat in de query :fname, :fname1 etc

dus ook
$stmt->bindParam(':fname', $fname, PDO::PARAM_STR);
$stmt->bindParam(':fname1', $fname1, PDO::PARAM_STR);

ik hoop trouwens dat je niet je hele code bouwt met $fname1 tm $fname9999 want dat maakt je code redelijk onbegrijpelijk.

Want: wat zit er nu in $fname4 op regel 41? Dan moet je helemaal naar boven terug scrollen.

Kies zinnige namen (liefst ook in probeersels en tests)

Thomas van den Heuvel

02-09-2015 14:30

Je kunt trouwens die typehints achterwege laten, daar doet PDO (ingeval van MySQL, weet niet hoe het bij de andere drivers zit) volgens mij helemaal niets mee. Daarnaast als je in PDO een variabele echt als integer wilt behandelen dan moet deze variabele van het type int zijn, anders fietst PDO hier alsnog quotes omheen (de enige manier om hier achter te komen is ook door het loggen van queries, een andere mogelijkheid is er niet), of je nu aangeeft dat het PARAM_INT moet zijn, of niet. Tegelijkertijd zal een variabele $whatever = '12' altijd als string behandeld worden, of deze waarde nu een "numeriek voorkomen" heeft of niet.

Reageren

Inloggen om te reageren