在我之前的动态调用后函数中,从变量Sven传递参数指出,我的代码容易受到本地文件包含的攻击。我做了一些修改以防止LFI。这够了吗?还是我应该担心?
if ($_SERVER['HTTP_X_REQUESTED_WITH'] !== "XMLHttpRequest")
{
echo "Error";
exit();
}
$req = explode("_",$_POST['req']);
/*
User input should always be escaped
using preg_quote before being used in a regexp pattern.
Thanks bwoebi
*/
$className = preg_quote($req[0]) . "Controller" ;
$methodName = $req[1];
$args= isset($_POST["data"]) ? $_POST['data'] : array();
$file = "application/controllers/" . $className . ".php" ;
if (!file_exists($file) || preg_match("/^[a-z]$/", strtolower($className)))
exit();
require_once $file;
$controller = new $className;
$result = call_user_func_array(array($controller, $methodName),$args );
echo json_encode($result);
另一个问题可能是用户可以从文件夹中调用任何控制器文件的公共方法。但据我所知,越来越多的框架在路由中使用domain.xy/controller/meethod/par模式,这也有同样的风险。(尽管在我的控制器中,我使用尽可能多的服务器端验证)
我想在ajax处理程序/路由器文件中加入一些身份验证。
// PSEUDO CODE
$user = new User();
// maybe bad practice to store the id session after authentication. Any comment on this?
$userGroup =$user->getUserGroupById($_SESSION["user"]);
$security = new Security();
$whiteList = $security->getWhiteList($userGroup);
//$whiteList is an array with the list of controllers the user may access
if (!in_array(className, $whiteList ))
exit();
欢迎任何意见、最佳实践示例!
它现在应该是安全的,因为您不允许在regex中包含恶意点和NUL字节。
但这仍然不是一个好的做法。你真的应该把它与你的白名单相匹配。正如你所说,它是安全的,只允许用户选择你允许用户选择的控制器。
tl;博士:使用你在伪代码中写的内容。
老实说,您的代码看起来一团糟。
-
在regexp模式中使用
$className
不是,那么为什么要用preg_quote()
来转义呢? -
为什么您先检查类文件是否存在,然后才检查类名是否允许?在我看来,这似乎是倒退的。(在这种情况下,它不应该造成任何实际的伤害,但在使用任何的输入之前,最好先进行输入验证。)
-
你的无效类名不是向后检查了吗?您的
preg_match("/^[a-z]$/", strtolower($className))
只有当
$className
仅由字母组成时,才会返回true。(另外,使用strtolower()
进行不区分大小写的匹配是一种效率很低的方法。)不应该是吗!preg_match('/^[a-z]$/i', $className)
甚至
preg_match('/[^a-z]/i', $className)
相反?
如果你修复了所有这些问题,你的代码可能会正常(假设你在这样做的时候没有引入任何新的bug)。
您可以通过创建一个名为application/BogusController.php
的文件来测试它,并通过传递../Bogus
作为类名来查看是否可以包含它。试着想想其他可能的变化,并测试它们。
在任何情况下,就像bwoebi注释一样,您的伪代码使用了更安全的方法,应该更难出错。我还建议你使用它。