Hello all ,
I am doing GSoC project on enhancing secure code review that is an automated tool to locate vulnerabilities in the code .
You can find my project page on this link :-Security Review and GSoC Project proposal here :- Proposal on Melange
I have an approach that can help in locating xss vulnerabilities .Please provide me your feedback,suggestions and your opinions about this approach.
Since xss data in database and other locations creates no problem,but if this data is displayed as output then it may creates severe problems . So what i think to make secure code review is that first we should list out all the functions in the module . Then in each such function we should search for functionCalls and then if we found any functionCall that displays data as output like theme(),drupal_set_message(),return etc ,then we should list out,what variables are used in this
output functionCall . Now we have a list of variables (parameter of functionCall ) whose data is going to be displayed as output . So now we will find from where these variables get their values .They might get their values from different functionCall or from another variable .Then again we should search from where these variables get their values and going on in the same recursion way we finally have the final value or final variable that is going to be displayed as output ..here we should check for different drupal filters like check_plain() or filter_xss if we found that value is coming from source that may provide xss vulnerable value ..
we can locate xss vulnerability using above approach in taxonomy_list module
Taxonomy_list Repository
In checking for functionCalls in function taxonomy_list_show() in line 296 we found that in line 384 their is an output functioncall drupal_set_title(),so we list out the variables that is used in drupal_set_title() which is $vocab_titles then while finding from where this variable is assigned a value we came across $vocab_titles[]=$vocab->name in line 360,and again searching from where $vocab come we find out it is coming from taxonomy_vocabulary_load() which might be a source to provide xss vulnerable value and in the path no where we found any of drupal filters used so we can say that this might be xss vulnerable .
What you will think about this approach towards locating xss ..Please provide me feedback,suggestions, drawbacks of this approach or how we can make this approach more efficient .
Thanks,
Udit
Comments
I think this is the approach
I think this is the approach that secure_code_review takes today and it seems to work well. The challenge came, afaik, in walking backwards (especially when you have to exit the current function) to find the source of the text and in identifying which text is a problem or not. It's not necessarily accurate to say that check_plain or filter_xss is always the appropriate/sufficient filter.
This is a very exciting project! Please keep asking questions and I hope you have good success.
knaddison blog | Morris Animal Foundation
ALGORITHM FOR FINDING SOURCE VALUES
Initiating :
*Scan all page callback function from hook_menu and stores these in an array $search_func_list.
*Store all functions names in an array $all_func_list which has global access.
*Store all output functionCalls to be scanned in functions in an array $function_calls like drupal_set_title() or theme().
Code Starts Running as follows:
For each function in $search_func_list (example -> let it takes function_one())
It takes a value from $function_calls (ex-> let it took drupal_set_title()) and gets all the functionCalls of the this type (drupal_set_title) from inside the function function_one()
These each of the functionCalls may contain both variable or another functionCall
ex-> drupal_set_title(variable_get($one),$two,function_two()) ;
3.1. It takes all the variables from inside the drupal_set_title() here it takes $one and $two and searches inside function_one() where they are last assigned. If these last assigned variables contains another variable as a value ex-> $one = $three then it again repeat the procedure of finding where three is last assigned and get the source value.If the value is inside any conditional statement like if,else_if then it also takes the value that is assigned to variable before the conditional as source .Since their might be conditions that if loops fails and we get the value assigned before it .Also it takes all values assigned to variable inside each of if,else if,else .Any of these value may come as a result.so it takes all of them as a source .If it founds any of functionCall assigned a variable (ex-> $a=function2()),then it will go to the step 3.1 to repeat the procedure .
3.2. Then it takes all the functionCalls from inside the function and searches for this functionCall in the $all_func_list .If it founds a value in $all_func_list then it will go inside that function and and it only scans for type return functionCalls inside this function and enter this function name in the list $to_be_searched array.If it not finds any functionCall in $all_func_list .Then it will save this functionCall as one the source .For ex-> First it takes variable_get() and found that it is not in $all_func_list,so it saves it as one of the source.Then it takes function_two() it now finds a value in $all_func_list .Now it goes inside the function_two() and searches for all of the return statements .Now it will go to step 3 and repeat the procedure.
3.3 For each functions in the $to_be_searched array Go to Step 2 and repeat the procedure for all functionCalls except return.
In this way we will traverse complete file and reach to source variables.
Please give your feedback and suggestions.
Thanks,