Jag håller på och tar fram en escape-funktion för SQL-strängar.
Vad tror ni om nedanstående funktion?
function safe_string_escape($str)
{
$len=strlen($str);
$escapeCount=0;
$targetString='';
for($offset=0;$offset<$len;$offset++) {
switch($c=$str{$offset}) {
case "'":
// Escapes this quote only if its not preceded by an unescaped backslash
if($escapeCount % 2 == 0) $targetString.="\\";
$escapeCount=0;
$targetString.=$c;
break;
case '"':
// Escapes this quote only if its not preceded by an unescaped backslash
if($escapeCount % 2 == 0) $targetString.="\\";
$escapeCount=0;
$targetString.=$c;
break;
case '\\':
$escapeCount++;
$targetString.=$c;
break;
default:
$escapeCount=0;
$targetString.=$c;
}
}
return $targetString;
}
Men varför...!? Varför inte använda mysql_real_escape_string eller ännu hellre prepared statements? Men om du nu vill veta så har din rutin två stora problem som kommer sig av att du inte ersätter \ med \\.
\ tolkas i sig som ett escape-tecken av MySQL's tolk. Det innebär t ex att \\ passerar obehindrat och lagras som \ medan \n också passerar obemärkt och blir till en riktig radbrytning i strängen som lagras i databasen. Kan t ex ställa till problem om någon försöker spara en snutt C-kod.
Fast koden är väl säker mot injektioner åtminstone? Nej! För vad händer om du sätter en backslash som sista tecken i en sträng?
$p1 = safe_string_escape('\\');
$p2 = safe_string_escape(' OR ... ');
$sql = "SELECT * FROM urk WHERE bla = '$p1' AND fy = '$p2';";
Frågan blir till:
SELECT * FROM urk WHERE bla = '[U]\' AND fy = [/U]' OR ... ';
(Det som tolkas som en sträng av MySQL är understruket.)
Visserligen är det svårare än vanligt eller eventuellt omöjligt för en attackerare att skapa en attack som inte leder till syntaxfel, men bara det faktum att det teoretiskt sett finns en lucka är dåligt!
Tack för att du tog dig tid att svara. Det du säger är att det jag föreslog gör det lite svårare att genomföra en attack men att det inte ger ett fullgott skydd. Då är det inte ett spår som jag kommer att jobba vidare på.
Jag har läst om mysql_real_escape_string-funktionen. Har du något förslag på hur jag ska implementera den? I nuläget har jag en en site med väldigt många SQL-frågor. Jag vill försöka hitta en medelväg som gör att jag ökar skyddet samtidigt som det inte leder till att jag behöver skriva om samtliga SQL-frågor (om det nu är vad det skulle innebära).
Som ovanstående talare var inne på,
använd prepared statements, du slipper allt jidder med escape m.m
Exempel med PDO:
$this->db = new PDO('mysql:host=localhost;dbname=test', 'root', 'sa');
$stmt = $this->db->prepare('SELECT data FROM ourTable WHERE id = : id');
$stmt->execute( array(':id' => $_GET['id']) );
$rows = $stmt->fetchAll();
Och skriv en generell databas klass så slipper du skriva om alltför mycket med PDO statements m.m,
exempel:
$db = new DB($pdo);
$result = $db->getResult('Select * from WHERE id = ? ', array(':id' => $_GET['id']) );
Jag har läst om mysql_real_escape_string-funktionen. Har du något förslag på hur jag ska implementera den? I nuläget har jag en en site med väldigt många SQL-frågor. Jag vill försöka hitta en medelväg som gör att jag ökar skyddet samtidigt som det inte leder till att jag behöver skriva om samtliga SQL-frågor (om det nu är vad det skulle innebära).
Själva SQL frågorna behover du oftast inte skriva om, det du behöver göra är att köra strängen med SQL frågan genom mysql_real_escape_string() funktionen, precis som du skulle behöva göra med din egna funktion.
Alltså
$sql="select * from foobar" blir $sql=mysql_real_escape_string("select * from foobar";
Eller
$resultat=mysql_query("select * from foobar") blir $resultat=mysql_query(mysql_real_escape_string("select * from foobar"))
$sql="select * from foobar" blir $sql=mysql_real_escape_string("select * from foobar";
Eller
$resultat=mysql_query("select * from foobar") blir $resultat=mysql_query(mysql_real_escape_string("select * from foobar"))
Näe, det där blir fel... Man måste escapea de individuella strängarna och sedan stoppa in dem i frågan. Annars kommer man escapa tecken som man faktiskt vill använda i frågan!
Jag hatar (eller inte ;) ) att vara så petig, men du har fel igen. Strängen måste sitta inom snuttar, även om det handlar om ett nummer! Vad händer annars om jag som användare gör
post.php?id=42 or 1=1
för att ta ett enkelt exempel. (Jo, alla inlägg listas ut.)
Alltså:
$id=mysq_real_escape_string($_POST['Id']);
mysql_query("select * from foobar where Id='$id' ");
Undantaget är om du uttryckligen konverterar variabeln till ett nummer eller försäkrar sig om att det är ett nummer först. Dock finns det inga nackdelar att även i det fallet göra både och. En bra strategi kan vara att dela upp dessa två saker i olika funktioner. Typ:
if(is_numeric($_POST['Id'])){
getPost($_POST['Id']);
}else{
printError('Invalid ID!');
}
function getPost($id){
$id=mysq_real_escape_string($id);
mysql_query("select * from foobar where Id='$id' ");
// ...
}
Undantaget är om du uttryckligen konverterar variabeln till ett nummer eller försäkrar sig om att det är ett nummer först.
Man måste självklart kontrollera vad det är för data som kommer in från en formulär oavsett vad man gör med det.
Är det en integer, kontrollera att det är en integer och om det finns gränser kontroller att talet är inom dessa gränser.
Gillar inte att man i bakgrunden konverterar saker om användaren skriver in fel, ha istället bättre kontroller att heltalsfält bara innehåller heltal och inget annat, skriver användaren fel påpeka detta för användaren och be användaren rätta till det.
Samma sak med datum, personnumer ... allt detta skall göras innan man börjar använda informationen i skript/program.
Vad bra att jag fick all denna fakta bekräftad om vilket arbete som krävs för att få till en säker site :) Då kommer jag påbörja migreringen över till vBulletin direkt. Jag har redan idag migreringsskript framtagna som täcker 60% av det innehåll som ska flyttas. Helt klart bättre att lägga energin där. Eller tycker ni inte det?
Att slå på Magic Quote på servern, är det ens att tänka på? Blir säkerheten något bättre som skulle kuna göra att sidan kan köras vidare under en interimsperiod?
Gillar inte att man i bakgrunden konverterar saker om användaren skriver in fel, ha istället bättre kontroller att heltalsfält bara innehåller heltal och inget annat, skriver användaren fel påpeka detta för användaren och be användaren rätta till det.
Samma sak med datum, personnumer ... allt detta skall göras innan man börjar använda informationen i skript/program.
När det gäller data som ska matas in i databasen, absolut. När det gäller att t ex hämta något utifrån ett id kan man nöja sig med att escapea och lägga inom fnuttar. Vare sig frågan blir
WHERE id = '28347237482383'
eller
WHERE id = 'Gargelblorf'
eller
WHERE id = '\' OR \'\' = \''
så blir resultatet noll rader ut från databasen som sedan kan presenteras som "sidan kan inte hittas" eller dylikt.
Att slå på Magic Quote på servern, är det ens att tänka på? Blir säkerheten något bättre som skulle kuna göra att sidan kan köras vidare under en interimsperiod?
Nej. Du ska glömma att magic_quotes finns, den funktionen är idiotisk.
