Skip to content
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

[WIP] Fix false-negatives and false-positives #99

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
3b4bf1b
Make a false_negatives directory with the issues from the 11/16/17 ev…
KevinHock Mar 29, 2018
153501f
Made a folder example/november_2017_evaluation with a directory for f…
KevinHock Mar 30, 2018
104cf80
Fix the foddy infinite loop
KevinHock Mar 31, 2018
4156984
Delete the infinite_loop folder since it is fixed
KevinHock Mar 31, 2018
97074f3
Make source_nested_in_sink.py test and pass it by making .args = list…
KevinHock Apr 4, 2018
98b49b6
Make a test for if_else_in_sink
KevinHock Apr 5, 2018
ff1bc89
Fix double import trap https://stackoverflow.com/questions/43393764/p…
KevinHock Apr 5, 2018
6c7a8d2
[False-negs] Problem illustration commit
KevinHock Apr 10, 2018
e9dd5e4
[comments] w/r/t ConnectStatements
KevinHock Apr 11, 2018
63c3f96
save current work emergency
KevinHock May 10, 2018
8367f68
save current work emergency
KevinHock May 10, 2018
2578c29
Merge branch 'master' into fix_oakie_false_negative
KevinHock Jun 21, 2018
6226a77
Added changes back to file that git merge missed
KevinHock Jun 21, 2018
62e7420
Fix whitespace
KevinHock Jun 21, 2018
8b72e52
Fixed some imports that got broken by the merge, indented EXPECTED in…
KevinHock Jun 21, 2018
d42243c
Delete tests/vulnerabilities_test.py that I accidentally added, Fix C…
KevinHock Jun 21, 2018
695bbde
Mostly finished BoolOp, IfExp and expr_star_handler
KevinHock Jul 4, 2018
7ecf462
Handle BoolOp inside of BoolOp, Make ConnectExpressions a class, Fix …
KevinHock Jul 5, 2018
1fa04e8
Finished IfExp, found bug in expr_star_handler
KevinHock Jul 6, 2018
4081900
Fix case where false-positives could occur by making the node before …
KevinHock Jul 7, 2018
c6d9e62
Add vulnerable_code_with_expressions/last_var_in_and_is_tainted and l…
KevinHock Jul 10, 2018
4d2ad46
Change visit_IfExp to handle .test better, change expr_cfg_test.py to…
KevinHock Jul 10, 2018
5b1f262
Add example of connecting_control_flow_exprs_in_connect_expressions.py
KevinHock Jul 10, 2018
b85c072
Things I did on 7/26/18
KevinHock Jul 26, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@app.route('/encode_tainted')
def encode_tainted():
state = request.args.get('state', '')
next = base64.urlsafe_b64decode(state.encode('ascii'))
return redirect(next)
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
"""
This is very similar to one of the false-negatives found in our November 16th/17th 2017 evaluation
http://pyt.readthedocs.io/en/latest/past_evaluations.html#november-16-17th-2017
"""
import scrypt


@app.route('/login', methods=['GET', 'POST'])
def login():
# 2 bool op
# print('foo')
# x = 5
# laun = request.args.get('Laundry')
laun = 'beep'
# return redirect(request.args.get('The') or request.args.get('French') or 'laun' and 'crazy')
# return redirect(request.args.get('The') or request.args.get('French') or request.args.get('Laundry') and 'crazy')

# works
# return redirect(request.args.get('The') or request.args.get('French') or 'crazy' and request.args.get('Laundry'))

# return redirect(request.args.get('The') if 1==2 else request.args.get('French') if 'foo' else 'crazy')
# return redirect(request.args.get('The') if 1==2 else request.args.get('French') if 'foo' else request.args.get('Aces'))

# return redirect(request.args.get('The') if 'hey' or 'you' else request.args.get('French') if 'foo' else 'c' and request.args.get('Aces'))
return redirect(request.args.get('The') if 'hey' or 'you' else request.args.get('French') if 'foo' else request.args.get('Aces') and 'c')


# works
# return redirect('crazy' and request.args.get('Laundry') )


# return redirect(request.args.get('The') if 1==2 else request.args.get('French'))
# return redirect(request.args.get('The') if 1==2 else laun)

# return redirect(request.args.get('The'))


# return redirect(request.args.get('The') if 1==2 else 'foo')
# return redirect('beep', request.args.get('The'))
# return redirect(request.args.get('The') if 1==2 else 'beep')
# return redirect(request.args.get('The'))
# return 'The'
# return redirect('The')


# 2 IfExp and a string
# return redirect(request.args.get('The') if 1==2 else request.args.get('French') if 2==3 else 'Laundry')
# 3 IfExp
# return redirect(request.args.get('The') if 1==2 else request.args.get('French') if 2==3 else request.args.get('Laundry'))

# All 3
# return redirect(request.args.get('The') if 5==5 else url_for('French'), request.args.get('Laundry') or url_for('Sushiritto'), scrypt.encrypt('Mixt'))
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@app.route('/index_of_tainted')
def index_of_tainted():
state = request.args.get('state', '')
next = base64.urlsafe_b64decode(state).split(';')[1]
return redirect(next)
20 changes: 20 additions & 0 deletions example/november_2017_evaluation/false_negatives/oakie_auth.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
"""
One of the false-negatives found in our November 16th/17th 2017 evaluation
http://pyt.readthedocs.io/en/latest/past_evaluations.html#november-16-17th-2017
"""

@app.route('/auth_callback')
def auth_callback():
error = request.args.get('error', '')
if error:
return 'Error: ' + error
state = request.args.get('state', '')
if not is_valid_state(state):
abort(403)
next = base64.urlsafe_b64decode(state.encode('ascii')).split(';')[1]
remove_state(state)
code = request.args.get('code')
id_token, access_token = get_tokens(code)

session['email'] = id_token['email']
return redirect(next)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
"""
This is very similar to one of the false-negatives found in our November 16th/17th 2017 evaluation
http://pyt.readthedocs.io/en/latest/past_evaluations.html#november-16-17th-2017
"""

@app.route('/login', methods=['GET', 'POST'])
def login():
return redirect(request.args.get("next") or url_for("index"))
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
"""
This is very similar to one of the false-negatives found in our November 16th/17th 2017 evaluation
http://pyt.readthedocs.io/en/latest/past_evaluations.html#november-16-17th-2017
"""


@app.route('/login', methods=['GET', 'POST'])
def login():
return redirect(request.args.get("next"))
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@app.route('/split_tainted')
def split_tainted():
state = request.args.get('state', '')
next = base64.urlsafe_b64decode(state).split(';')
return redirect(next)
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
@app.route('/image', methods = ['GET'])
def get_image():
url = request.args.get('url', '')
if not url:
abort(400)

if is_image(url):
Copy link
Collaborator Author

@KevinHock KevinHock Apr 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one seems bypass-able, so not a false-positive, but it easily could have been, in order to do this well we would need to be able to solve predicates, via like Z3str3 or something. I feel these are acceptable for the foreseeable future.

return redirect(url)

def is_image(url):
image_extensions = ['.jpg', '.gif', '.png', '.jpg', '.bmp', '.webp', '.webm']
extension = url[url.rfind('.'):]
return extension in image_extensions
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
"""A minimized version of flask_expand_url to illustrate the question of 'what args being tainted means a vulnerability?'"""

from Flask import g

@app.route('/<david>')
def expand_url(david):
foster = query_db('select url_long from urls where david = ?', [david])


def query_db(query, args=()):
wallace = g.db.execute(query, args)
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
from Flask import g

@app.route('/<url_short>')
def expand_url(url_short):
"""Check for url in DB"""
result = query_db('select url_long from urls where url_short = ?',
[url_short], one=True)
if result is None:
return redirect(url_for("index"))
else:
link = result['url_long']
return redirect(link)


def query_db(query, args=(), one=False):
"""Queries the database and returns a list of dictionaries."""
cur = g.db.execute(query, args)
rv = [dict((cur.description[idx][0], value)
for idx, value in enumerate(row)) for row in cur.fetchall()]
return (rv[0] if rv else None) if one else rv
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
"""
One of the false-positives found in our November 16th/17th 2017 evaluation
http://pyt.readthedocs.io/en/latest/past_evaluations.html#november-16-17th-2017
"""
@app.route('/client/passport', methods=['POST','GET'])
def client_passport():
code = request.args.get('code')
uri = 'http://localhost:5000/oauth?response_type=%s&client_id=%s&redirect_uri=%s' %(code,client_id,redirect_uri)
return redirect(uri)
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
"""
This is very similar to one of the false-negatives found in our November 16th/17th 2017 evaluation
http://pyt.readthedocs.io/en/latest/past_evaluations.html#november-16-17th-2017
"""

redirect(
request.args.get('Laundry') and 'crazy'
or
request.args.get('French') and 'foo'
or
request.args.get('The')
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""
This is very similar to one of the false-negatives found in our November 16th/17th 2017 evaluation
http://pyt.readthedocs.io/en/latest/past_evaluations.html#november-16-17th-2017
"""

redirect(request.args.get('The') if 'hey' or 'you' else request.args.get('French') if 'foo' else request.args.get('Aces') and 'c')
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""
This is very similar to one of the false-negatives found in our November 16th/17th 2017 evaluation
http://pyt.readthedocs.io/en/latest/past_evaluations.html#november-16-17th-2017
"""

redirect(request.args.get('The') if 'hey' or 'you' else request.args.get('French') if 'foo' else 'c' and request.args.get('Aces'))
4 changes: 2 additions & 2 deletions pyt/cfg/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ So as you'll see, there is a `visit\_` function for almost every AST node type.
The two most illustrative functions are `stmt_star_handler`_ and expr_star_handler. expr_star_handler has not been merged to master so let's talk about `stmt_star_handler`_.


Handling an if: statement
Handling an if: statement
=========================

Example code
Expand All @@ -43,7 +43,7 @@ This is the relevant part of the `abstract grammar`_
.. code-block:: python

If(expr test, stmt* body, stmt* orelse)
# Note: stmt* means any number of statements.
# Note: stmt* means any number of statements.


Upon visiting an if: statement we will enter `visit_If`_ in `stmt_visitor.py`_. Since we know that the test is just one expression, we can just call self.visit() on it. The body could be an infinite number of statements, so we use the `stmt_star_handler`_ function.
Expand Down
Loading