这个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