-
-
Notifications
You must be signed in to change notification settings - Fork 312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
关于ast分析的一些问题 #188
关于ast分析的一些问题 #188
Conversation
This pull request introduces 5 alerts when merging 3ea91a6 into 1ee36f1 - view on LGTM.com new alerts:
|
This pull request introduces 1 alert and fixes 8 when merging 01b0d86 into 1ee36f1 - view on LGTM.com new alerts:
fixed alerts:
|
This pull request introduces 4 alerts when merging 1800e25 into 1ee36f1 - view on LGTM.com new alerts:
|
不用修语法问题没事。我看一下这几个点有没有问题再合并 |
发现语法问题需要测试很多样本才能发现问题。。 |
This pull request introduces 5 alerts when merging 7925485 into 1ee36f1 - view on LGTM.com new alerts:
|
最后那个你看看有没有问题 |
关于这部分我认为不一定能改,因为现在的递归逻辑就是向上回溯,如果改成all_nodes,这里可能会导致执行顺序存在问题。这个问题我原本是打算修改底层逻辑来修复的 |
Result=[] | ||
for i in range(0,len(result)): | ||
Result.append(result[i]['code']) | ||
if 1 in Result: # 只要有一个函数参数可控就返回,chain都是一样的 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里已经return了,你说是不是和我那个写法没什么区别。
这里底层设计有点儿问题当时
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个地方我看到每个可控参数的chain都是一样的,所以就直接返回了一个
@@ -72,7 +72,7 @@ def main(self, regex_string): | |||
regex string input | |||
:return: | |||
""" | |||
sql_sen = check_tuple(regex_string[0]) | |||
sql_sen = str(regex_string[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里为啥要改呀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
递归逻辑没问题啊,我就是改了一下传入function_back函数的节点为文件所有节点,使回溯函数定义从文件最下面那个节点开始 |
其他基本上没问题,这几个改了我测试一下就合并 |
唔,怎么不见啦..我明天自己改下吧:< |
还没来得及改,有些地方好像是有点粗糙,本来准备后面再改的 |
关于你第一部分的改动,我仔细研究了一下,我觉得完全没有必要吧。如果出现
这种代码,岂不是直接判断在参数列表中就返回了,目前的逻辑是会遍历完整个函数再去判断参数的。 |
这个和我上面那个情形不一样吧,我上面那个是函数外进入函数内分析,这个本来就是函数内。没测试你说的,后面再看看 |
例如以下代码:
上面代码中当回溯到$d可控时应该将getFilter第一个参数标记为可控函数,然后再继续分析函数,而不是直接当作漏洞(测了一下当$d不可控时也不会分析函数,即使函数内可控)
处理赋值语句是functioncall的时候是把函数(函数返回值)当作新的变量(即污点),然后进入函数内回溯看返回值是否可控
但是当functioncall函数有参数时,分析参数是否可控的时候将本来是函数的param赋值为该参数,从而导致后面不会分析该函数
将该else分支的param参数修改为param_param,从而保证后面进入正常的函数分析流程
定义一个全局变量fun_call,当把函数赋值为参数时将fun_call赋值为True
回溯完函数并且该参数可控时,将该变量添加到可控变量列表中
因为涉及到传入调用函数的变量与该函数定义时的变量不同的情况,所以在function_back函数中先回溯该functioncall的参数并从可控函数列表中删除,然后对应到函数定义的相应位置的参数添加到可控函数列表中,回溯完函数之后再次判断fun_call并将参数从可控列表中删除
例如如下情形:
当param被赋值时,虽然将param赋值为新的参数再递归,但是没有赋值param_name参数,后面生成规则时是根据param_name来判断的,它最初匹配到关键代码
$value = call_user_func($filter, $value);
,此时的变量是$filter,而$filter是由$filters通过foreach遍历而来,此时$filters是新污点,但是最后的param_name还是$filter,影响最后的规则生成,所以param_name也增加了赋值。同时这里由于是递归,需要返回才能将最初的param_name和param参数覆盖例如一下代码:
传入function_back的节点是直接传的递归剩下的节点(向上递归回溯),如果函数定义在下面,那么将找不到而产生漏报。将anlysis_params函数的all_nodes(文件中所有节点)定义为全局变量,传入该函数节点时传all_nodes。
analysis函数中增加了php.Foreach、php.AssignOp、php.BinaryOp的处理;增加了对php.Trait、php.Foreach、php.Cast的处理;解决php.FunctionCall节点和php.Assignment下的php.FunctionCall节点不能处理多重函数调用
如果分析到一个参数可控或在函数参数中就立即返回,问题在于如果另一个参数也可控,并且在后来的回溯中也可控,那么这个参数在后面的话,这个漏洞就会漏报,修改了一下init_match_rule函数的规则,同时去掉了’不能匹配括号’,使其能匹配下面这种形式