Blackbox functions return tainted value (false positives) #45

Activity

1. KevinHock

   self-assigned this

   on May 18, 2017
2. Thalmann commented on May 21, 2017

   

   Thalmann

   on May 21, 2017

   Contributor

   More actions

   Hmm, i do not know if this even makes much sense.

   The first example about absolute import will not even run(ImportError).
   The second one will fail when we make the post.

   So i do not think we need to support analysis of code the python interpreter does not even understand.
3. StefanMich commented on May 21, 2017

   

   StefanMich

   on May 21, 2017

   Contributor

   More actions

   In my opinion the best solution would be to mark it as a vulnerability and in some way indicate that it is due to the name not being imported. That way the developer is able to disregard the vulnerability if he knows that he uses some dynamic magic to set the name
4. KevinHock commented on May 21, 2017

   

   KevinHock

   on May 21, 2017

   Last edited by KevinHock

   CollaboratorAuthor

   More actions

   I probably could have explained this better, say

   import subprocess
   from flask import Flask, render_template, request
   
   # This is a lib we can't possibly see inside of
   import scrypt
   
   
   app = Flask(__name__)
   
   @app.route('/menu', methods=['GET'])
   def menu():
       param = request.args.get('suggestion')
   
       # This is a function we can't possibly see inside of
       foobar = scrypt.encrypt('echo ' + param + ' >> ' + 'menu.txt', 'password')
       command = scrypt.encrypt('echo ' + param + ' >> ' + 'menu.txt', 'password')
       hey = command
       subprocess.call(hey, shell=True)
   
       with open('menu.txt','r') as f:
           menu = f.read()
   
       return render_template('command_injection.html', menu=menu)
   
   if __name__ == '__main__':
       app.run(debug=True)

   scrypt.encrypt could be any call from a library that we can't possibly know e.g. If all tainted args means a tainted return value.

   So we shouldn't report it as a vulnerability with as much confidence as everything else. We can talk more about the UI part when I make the PR on Monday or Tuesday.

   The way I did this was, to put the call in a set called blackbox_calls and then check in the assignment_with_call type function if the call is in blackbox calls. If it is, I add the assignment to blackbox assignments, which can be checked when sanitation is checked in vulnerabilities.py. This will get more complicated when I handle nested function calls but I'll write tests and incrementally handle things.
5. KevinHock commented on May 21, 2017

   

   KevinHock

   on May 21, 2017

   Last edited by KevinHock

   CollaboratorAuthor

   More actions

   Maybe one day we'll do it like this awesome paper but, we're not there yet.
6. KevinHock commented on May 21, 2017

   

   KevinHock

   on May 21, 2017

   Last edited by KevinHock

   CollaboratorAuthor

   More actions

   Maybe this update to the example file is better, thoughts? (Why would you exec an scrypt hash is a valid question but I didn't feel like running pyt until I find a better example.)
7. KevinHock

   closed this as completedin #63on Nov 12, 2017