Draft PR for Pysa Fuzzer

[esohel30]

Draft PR for the pysa fuzzer project 🚀

[arthaud]

Cool, this is a good start.

A few comments:

• Could we use type annotations in the script? See https://docs.python.org/3/library/typing.html. We try to annotate most of our python code at Meta. It should be fairly easy here, most arguments are str and the return value of most functions is also str
• I think this is not generating valid code, since a variable might be used before it is defined. Could you find a way to prevent that problem?
• There are many patterns that cannot be generated because generate_complex_expression is not truly recursive. For instance, generate_if_statement uses generate_assignment which itself uses generate_variable_expression. It will never generate a if condition that contains a function call, for instance. Do you have ideas on how to improve that? I can help if needed.
• Instead of implementing your own indent, just use textwrap.indent from the standard library
• Finally, most importantly: in the end, this always generate sink(source()) (where source and sink are one of the 6 functions). Pysa will likely find the flow 99.99% of the time since the whole code before that doesn't really matter. What we need is the source to actually flow through the randomly generated code. Then, the hard part is to either: only generate code where the flow is valid, or somehow have a way to execute the code to verify if the flow is valid (to see if Pysa agrees with it).

[esohel30]

Enhanced Pysa fuzzer by adding type annotations, ensuring variables are defined before use, making expression generation truly recursive, and using textwrap.indent for better code formatting. Added a defined_variables set to track declared variables, improving code validity. Despite these improvements, the fuzzer is still far from perfect and requires further refinement to enhance code generation diversity and flow validation. Will continue to work on it extensively! Might even approach it in a different manner now that I have been playing around with a it for a bit.

[arthaud]

Cool, another round of feedback:

• I believe this never generates a valid flow, since sink_var is always the result of calling get_next_variable called after generating the expression, so that variable is never assigned. To fix that, you need the whole code generation to know the source_var and sink_var and have an assignment at some point.
• There is a bug where this can generate x = x since to_var is created before the right hand side expression is generated
• What if get_next_variable runs out of variable? We should probably use variable names such as a, .., z, aa, .., az, etc.
• Can we find a way to avoid using mutable global variables (e.g, defined_variables and current_var_index)? This is a problem if we want to generate several tests by calling generate_source_to_sink multiple times. A solution would be to use a class with attributes. All the functions to generate expressions could be methods of that class.
• It is still not fully recursive: generate_arithmetic_expression is never called, and generate_assignment only uses generate_variable_expression. This will never generate x = foo() or x = y + z + 1. Here is an idea: you should first differentiate between expressions (return a value) and statements. The current generate_complex_expression generates statements, not expressions. Then, the actual generate_expression function should be recursive and accept a maximum depth. Pick a number, then if it's 0: generate a constant, if it's 1, generate an addition, by calling generate_expression recursively, if it's 2, generate a substraction, etc. if it's some number N, generate a function call, etc.
• It would be interesting to also generate expressions with lists (creating lists, adding an element to a list, accessing a list item) and dictionaries (dict).

I believe @alexkassil also has a different idea to generate code that always has a valid flow, feel free to ask him if you are interested.

[arthaud]

My feedback:

• This is definitely going into the right direction. fuzzer2 always generates code with a valid flow, which is great.
• The generated code is still not valid, unfortunately. Variables are sometimes integers, and sometimes strings, used interchangeably. This would lead to a runtime error since 'a' + 1 is not valid python code.
• While we do generate interesting code, the code we generate would always have all variables tainted. It would be interesting to have more flows with variables that are not tainted. We need to inject variables that are not constructed from "prev_var".
• The main downside of the approach in fuzzer2 is that we generate a limited set of patterns, and those are only combined by being concatenated. If pysa handles those patterns individually, it is likely that it will handle correctly all generated code. To fix that, I think we need to combine fuzzer.py and fuzzer2.py: each generate_xxx (e.g, generate_while_loop , generate_for_loop , etc.) should be recursive so it can have nested statemenets. Right now we only generate while loops that have {curr_var} += {prev_var}\ncounter += 1. It would be more interesting to have nested while loops, while loops with if conditions, etc.
• Have you tried running pysa on the generated code? Does it find the issue? The next step is to do that automatically in the script, and run until we find an example where pysa doesn't find the flow.

[alexkassil]

Hey @esohel30 , so the idea for this project is to automatically find false negatives - ie flows that should be security issues, but for whatever reason pysa doesn't find it.

The way to do this is to generate increasingly complex valid flows for pysa to find. Everything generated should be a valid security issue.

https://github.com/facebook/pyre-check/tree/main/source/interprocedural_analyses/taint/test/integration take a look at the tests here (in the .py files).

Here's an example: https://github.com/facebook/pyre-check/blob/main/source/interprocedural_analyses/taint/test/integration/source_sink_flow.py

from builtins import _test_sink, _test_source


def bar():
    return _test_source()


def qux(arg):
    _test_sink(arg)


def bad(ok, arg):
    qux(arg)


def some_source():
    return bar()


def match_flows():
    x = some_source()
    bad(5, x)

One issue in this file is the flow in match_flows() -> bad()

One way to always generate valid issues is to start with _test_sink(_test_source()), and then pick operations that make it so the flow is one more hop away.

For example, let's say you add 3 functionalities of mutations to the fuzzer:

1. Extra variable
2. Function call
3. if statement else clause

And now you randomly pick from those 3 elements 4 times to get [1, 2, 3, 2].

Applying those mutations step by step gets you:

f1():
  x = test_source()
  _test_sink(x)

def f2():
  x = test_source()
  f2(x)

def f1(x):
  _test_sink(x)

def f2(cond):
  x = test_source()
  if cond:
    pass
  else:
    f2(x)

def f1(x):
  _test_sink(x)

def f3(x, cond):
    x = test_source()
    f2(x, cond)

def f2(x, cond):
  if cond:
    pass
  else:
    f2(x)

def f1(x):
  _test_sink(x)

Continuing adding more and more single hop transformations that preserve the flow will make the fuzzer be able to generate all the valid flows present in https://github.com/facebook/pyre-check/tree/main/source/interprocedural_analyses/taint/test/integration - I think for simplicity do not worry about any modelling other than _test_source and _test_sink

[alexkassil]

Looking at the code, here is how I recommend you change it high level going forward:

1. Look through https://github.com/facebook/pyre-check/tree/main/source/interprocedural_analyses/taint/test/integration and pick a few simpler cases to try to model with the fuzzer. Say somewhere around 5 cases
2. If you exhaustively generate all 4 length mutations, that's 555*5, or 625 valid but different pysa flows.
3. run pysa on all of them and validate pysa catches those errors

[facebook-github-bot]

@arthaud has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@arthaud merged this pull request in 99a07a2.

[alexkassil]

Congrats and well done @esohel30 !

[arthaud]

Thanks for the hard work! We have finally merged this and found a few false negatives (see upcommit commits).