這個PHP代碼是否安全？

Question

只是一個簡單的問題：以下PHP代碼是否安全？還有什麼你認爲我可以或應該添加？

$post = $_GET['post']; 

    if(is_numeric($post)) 
    { 
     $post = mysql_real_escape_string($post); 
    } 
    else 
    { 
     die("NAUGHTY NAUGHTY"); 
    } 

    mysql_select_db("****", $*****); 

    $content = mysql_query("SELECT * FROM tbl_***** WHERE Id='" . $post . "'"); 

Answer 1

這有點粗糙，但我沒有立即看到任何會導致任何嚴重問題的事情。您應該注意，根據文檔，在is_numeric()內接受十六進制符號。你可能想要使用is_int()或施放它。而爲了清楚起見，我建議使用參數化查詢：

$sql = sprintf("SELECT col1 
       FROM tbl 
       WHERE col2 = '%s'", mysql_real_escape_string($post)); 

在這種情況下，$post將在作爲%s的值傳遞。

Answer 2

is_numeric將對諸如'0xFF'之類的十六進制數字返回true。

編輯：要解決這個問題，你可以這樣做：在片段here on php.net更多信息

sprintf('%d', mysql_real_escape_string($post, $conn)); 
//If $post is not an int, it will become 0 by sprintf 

看。

Answer 3

在這種特殊情況下，我猜想is_numeric可以幫助您避免SQL注入（儘管您仍然可以打破SQL語句，請參閱Alex的答案）。不過，我真的覺得你應該考慮使用參數化查詢，因爲（又名準備好的發言）：

1. 使用非數字類型的參數時，他們甚至會保護你
2. 你不要冒險忘記輸入環衛你增加更多的參數
3. 您的代碼將是一個更容易讀寫

下面是一個例子（$db是PDO連接）：

$stmt = $db->prepare('SELECT * FROM tbl_Persons WHERE Id = :id'); 
$stmt->execute(array(':id' => $_GET['post'])); 
$rows = $stmt->fetchAll(); 

有關PHP參數化的SQL語句的更多信息，請參閱：

• Best way to stop SQL Injection in PHP

Answer 4

我認爲它看起來OK。

訪問數據庫時，我總是使用查詢綁定，這可以避免如果我忘記逃離我的字符串時出現的問題。

Answer 5

你有正確的想法，但is_numeric（）可能不像你想要的那樣工作。

你可以做個試驗：

<?php 
$tests = Array(
     "42", 
     1337, 
     "1e4", 
     "not numeric", 
     Array(), 
     9.1 
     ); 

foreach($tests as $element) 
{ 
    if(is_numeric($element)) 
    { 
     echo "'{$element}' is numeric", PHP_EOL; 
    } 
    else 
    { 
     echo "'{$element}' is NOT numeric", PHP_EOL; 
    } 
} 
?> 

結果是：

'42' is numeric 
'1337' is numeric 
'1e4' is numeric 
'not numeric' is NOT numeric 
'Array' is NOT numeric 
'9.1' is numeric 

1E4可能不是東西，如果你在尋找什麼是通常被稱爲一個數值，您的SQL Server理解。從SQL注入的角度來看，你很好。

Answer 6

您沒有將連接資源傳遞給mysql_real_escape_string（）（但您似乎是通過mysql_select_db（）來實現的）。連接資源和其他東西存儲連接字符集設置哪個might affect the behavior of real_escape_string()。
要麼不傳遞任何資源或（最好）通過它總是，但不要讓它比通過混合兩者傳遞資源更糟糕。

我的書中的「安全性」還包括代碼是否可讀，「易於理解」以及是否「直截了當」。在這個例子中，你至少必須向我解釋爲什麼當你在SELECT查詢中將id作爲字符串對待時，爲什麼你有!numeric -> die分支。我反駁（作爲例子表示，可能是錯在你方面）將是「何苦的SELECT就不會返回任何記錄一個非數字ID？」，這降低了代碼

if (isset($_GET['post'])) { 
    $query = sprintf(
    "SELECT x,y,z FROM foo WHERE id='%s'", 
    mysql_real_escape_string($_GET['post'], $mysqlconn) 
); 
    ... 
} 

這會自動消除您可能遇到的麻煩，因爲is_numeric（）不像您期望的那樣運行（如其他答案中所述）。

編輯：關於使用die()經常/在生產代碼早期有些話要說。測試/示例代碼很好，但在實時系統中，您幾乎總是希望將控制權交還給周圍的代碼，而不是僅僅退出（這樣您的應用程序可以優雅地處理錯誤）。在開發階段，您可能需要提前退出或進行更多測試。在這種情況下，請查看http://docs.php.net/assert。
您的示例可能有資格獲得斷言。如果斷言被停用，它不會中斷，但它可能會給開發人員更多的信息，說明爲什麼在傳遞非數字參數時它不按預期工作（由其他開發人員）。但是你必須小心地將必要的測試與斷言分開;他們不是銀彈。
如果你覺得is_numeric（）是一個重要的測試，你的函數（？）可能會返回false，拋出一個異常或其他信號來表示條件。但是對我來說，早死是一種簡單的方式，有點像一個無知的負鼠，「我不知道現在該做什麼，如果我死了也許沒有人會注意到」;-)

強制提示以準備對帳單：http://docs.php.net/pdo.prepared-statements