插件開發經驗之安全風險.避免審核駁回

我是小站長

交流探討應是好事，但有些重復問題老是不斷的問，弄的我都煩了。為此，整理一些開發者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! 應用中心應用審核規范”，避免違規。否則，遭罪的是自己
以上是個人經歷及經驗之談，有不當之處請指正。

本篇屬教程類，希望版主能移動到“插件教程”子版塊中。