插件开发经验之安全风险.避免审核驳回

我是小站长

交流探讨应是好事，但有些重复问题老是不断的问，弄的我都烦了。为此，整理一些开发者Q友常见的问题，在此一并回答如下：
常有开发者的作品因数据库SUID(SELECT、UPDATE、INSERT、DELETE)的操作涉及安全风险而无法审核通过，可又一时搞不清所说的“安全风险”到底是哪些代码，不知所措。我来讲解一下，希望能有所帮助(当初我也经常遇到这类问题，随着经验积累，慢慢有所悟了)。

此类“安全风险”，一般是指对$_GET和$_POST有没有进行安全过滤处理(addslashes)。
搞清这个概念，问题就好解决了。举个自定义C::t方法的例子

$name  = $_GET['name'];//从地址栏或表单获取
$query = C::t('#mp#common_member')->fetch_by_username($name);
这里用了$_GET，由于该值将用于数据库操作，那么该值是否安全呢？
我们来看DZ的技术文库是怎么说的，经查"Discuz! X2.5新版架构优化说明"—>"程序底层架构"—>"function_core.php 减肥之术"—>"用户输入数据的处理"中，有两句话是这样说的：
$_GET和$_POST的值默认不做addslashes处理；
$_GET为$_GET和$_POST数组的合并，代码中统一使用$_GET取值;
显然，上面参与数据库操作的$name变量是不安全的。但事实真这样吗？
下面看看与之相关自定义的C::T方法是怎么写的吧。
------------------------
……
public fetch_by_username($name){
　　return DB::result_first('select * from %t where username=%s',array($this->_table,$name));
}
……
通过上面的代码，需要明白两件事：
①该方法调用了DB层封装的函数result_first；
②使用了%s对数组第二参数$name进行格式化处理；
对于①，我们再看DZ的技术文库，经查"Discuz! X2.5新版架构优化说明"—>"数据库DB层"—>"新增数据层：数据层的规范和约定"中，有句话是这样说的：
"DB层封装的函数实现了addslashes，个别直接写sql语句的需主意addslashes；"
通过查看source/discuz/discuz_database.php，我们可以找到该类定义的result_first方法，说明该方法是被封装在DB里的，所以基本确定$name是安全的。
至于为啥要查看discuz_database.php，而不是discuz_table.php，自己慢慢上下求索吧。
对于②，我们再看DZ的技术文库，经查"Discuz! X2.5新版架构优化说明"—>"数据库DB层"—>"原DB类的改进"—>"3、SQL语句format的支持"中，可以看到支持的格式化表，其中%s表示进行addslashes安全过滤处理。至此，我们可以完全确定上面的代码是安全可靠的了。
但是如果我将自定义函数修改如下：
……
public fetch_by_username($name){
　　return DB::result_first('select * from %t where username=%i',array($this->_table,$name));
}
……
public fetch_by_username($name){
　　return DB::query('select * from '.DB::table('xxx').' where username='.$name);
}
……
会怎样呢？留给大家思考。
------------------------

再看一个例子：
$query = DB::query('select * from '.DB::table('xxx').' where username='.$_GET['name']);
虽然DB封装了query函数，但该函数并未进行addslashes处理，而只是检验SQL语句里的每一个字符，严格的讲是不安全的。
另外，DZ的技术文库—>"Discuz! X2.5新版架构优化说明"—>"数据库DB层"—>"新增数据层：数据层的规范和约定"中规定："不能使用$_G、$POST、$GET等全局变量；"
其实，除了不能使用规定的三个全局变量外，$_REQUEST变量也不应该在SQL中使用！

那么，凡是$_GET都要进行add...处理吗？不一定，要看用在什么地方，一般来讲，参与数据库SUID操作的DB::query中都要进行处理。例如：
$name  = addslashes($_GET['name']);
$query = DB::query('select * from '.DB::table('xxx').' where username='.$name);
但要注意避免重复过滤，如
$name  = addslashes($_GET['name']);
$query = DB::query('select * from '.DB::table('xxx').' where username='.addslashes($name));

综上：
1、使用C::t方法的，要注意相关参数是否用%s进行了格式化
2、无论是否C::t方法，使用DB::query(...)的,必须进行add...处理
3、避免重复过滤
4、尽量避免在SQL中使用禁用的全局变量，因为有时获取的变量值有违初衷，不是想要的结果
5、建议使用C::t方法
6、尽量了解、熟知、掌握DZ技术文库
7、尽量了解、熟知、掌握discuz_database.php、discuz_table.php等文件内容

有同行曾问我，为啥X2.5以后不再支持$_G['gp_xx']，我也不知道，这个要问官方。但既然不支持，最好就不要用，否则你还得写个说明，告知用户开启兼容的方法，这不是给自己找麻烦嘛。

还有同行问我，C::t方法有啥好处？我认为好处就是对象清楚、对SQL进行了格式化处理、降低了安全风险，并且有利于维护。

也有一些用户问我为啥不再开发X2的插件了，这个很简单，DZ的技术文库—>"Discuz! X2.5新版架构优化说明"—>"程序底层架构"—>"function_core.php 减肥之术"—>"用户输入数据的处理"中，第一句话是这样说的："Discus! X2.5之前版本对$_GET和$_POST的值默认是进行addslashes处理，函数在使用此值时信任外部数据的安全性，但这样处理的弊端是容易生产二次注射的漏洞，为了防止此类漏洞的产生，函数必须不信任任何数据外部数据的安全性"。因此，我对X2及以前的版本安全性是不信任的，出于对用户负责，所以不再开发X2的插件了。

另外，忠告开发者，尤其是新手，要想提高插件审核通过率，应做到以下几点：
①养成良好规范的代码书写习惯，可读性强。那种一大堆乱码似的代码，人见人烦
②从审核者的角度着想，该注释的地方加注释，便于审核者理解此处用意，避免引起误解而被踢回。与人方便与己方便
③清理垃圾文件和代码，让审核者省时省力，提高审核进度。只有好处没有坏处
④没事就多看看“开发文档”，尤其是“Discuz! 应用中心应用审核规范”，避免违规。否则，遭罪的是自己
以上是个人经历及经验之谈，有不当之处请指正。

本篇属教程类，希望版主能移动到“插件教程”子版块中。