Discuz! 官方站

 找回密码
 立即注册

QQ登录

只需一步,快速开始

搜索
查看: 2693|回复: 7

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

[复制链接]
发表于 2013-9-1 01:02:17 | 显示全部楼层 |阅读模式
本帖最后由 要命的酒鬼 于 2013-9-1 01:04 编辑

交流探讨应是好事,但有些重复问题老是不断的问,弄的我都烦了。为此,整理一些开发者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! 应用中心应用审核规范”,避免违规。否则,遭罪的是自己
以上是个人经历及经验之谈,有不当之处请指正。

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

评分

2

查看全部评分

发表于 2013-9-1 09:28:27 | 显示全部楼层
本帖最后由 1314学习网 于 2013-9-1 09:44 编辑

支持下分享经验

文库地址:http://dev.discuz.org/wiki/index ... 4.E5.A4.84.E7.90.86

回复

使用道具 举报

发表于 2013-9-1 11:19:13 | 显示全部楼层
来看看!!学习
回复

使用道具 举报

发表于 2013-9-1 14:36:56 | 显示全部楼层
感谢分享
回复

使用道具 举报

头像被屏蔽
发表于 2013-9-1 18:38:54 | 显示全部楼层
提示: 作者被禁止或删除 内容自动屏蔽
回复

使用道具 举报

发表于 2013-9-3 21:47:31 | 显示全部楼层
感谢分享!!
回复

使用道具 举报

发表于 2015-3-2 14:21:25 | 显示全部楼层
学习顶起
回复

使用道具 举报

发表于 2015-3-2 19:47:32 | 显示全部楼层
学习了
回复

使用道具 举报

您需要登录后才可以回帖 登录 | 立即注册

本版积分规则

腾讯云商标服务特惠!快速注册低至270元/件起,更有续展/宽展+延伸一站式服务、全场代金券补贴

小黑屋|Discuz! 官方站 ( 粤B2-20090059-165 )star

GMT+8, 2020-4-9 11:11

Powered by Discuz! X3.3

Copyright © 2001-2019 Tencent Cloud.

快速回复 返回顶部 返回列表