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

[Fall 2021] Step 3: Add a pyre validate-taint-config command and make errors in taintConfiguration.ml typed #82

Open
onionymous opened this issue Oct 13, 2021 · 3 comments
Assignees
Labels
Fall 2021 Issues related to the Pysa project for MLH Fellowship cohort of Fall 2021 step 3

Comments

@onionymous
Copy link
Collaborator

onionymous commented Oct 13, 2021

Outdated, please read later comment.

Pysa rules, sources, sinks and other taint information are specified in taint.config files. Multiple taint.config files can be specified in one project. When Pysa is run, it looks at all the taint.config files specified in the "taint_models_path" of the .pyre_configuration file for that project, and reads the rules, source/sink names, etc. from all of these files.

The goal of this project is to have some method to validate these taint.config files. This can be in the form of a Python script that we can run standalone, or something that is run at the start of invoking the Python client.

Some validation ideas to get started (feel free to think of more!):

  1. Make sure there are no duplicate warning codes. All Pysa rules should have a unique warning code. When there are multiple taint.config files, it can be confusing to keep track of the codes across different files, so we want to make sure no codes are repeated.
  2. Make sure the source/sink names used in rules exist.
  3. A regex validator for implicit string sources/sinks. Pyre/Pysa is written in OCaml and the regex engine used is re2. This is a non-backtracking engine which doesn't support a lot of features in other regex flavors like lookaheads, etc. (see https://github.com/google/re2/wiki/Syntax) so we want to make sure the regexes are valid re2 syntax. In addition, we want to make sure backslashes are escaped properly in these regexes, etc.
@onionymous onionymous added Fall 2021 Issues related to the Pysa project for MLH Fellowship cohort of Fall 2021 step 3 labels Oct 13, 2021
@abishekvashok abishekvashok self-assigned this Oct 14, 2021
@abishekvashok
Copy link

abishekvashok commented Oct 22, 2021

Okay, so I was looking at the ocaml source,
It seems we have the warning code uniqueness validator

let validate_code_uniqueness code =
if Hash_set.mem seen_rules code then
failwith (Format.sprintf "Multiple rules share the same code `%d`." code);
Hash_set.add seen_rules code
in

But we don't have that applied across multiple taint.config files (or do we?)

And for sources and sinks, we validate them via this function:

let parse_source ~allowed ?subkind name =
match
List.find allowed ~f:(fun { name = source_name; _ } -> String.equal source_name name), subkind
with
(* In order to support parsing parametric sources and sinks for rules, we allow matching
parametric source kinds with no subkind. *)
| Some _, None -> Ok (Sources.NamedSource name)
| Some { kind = Parametric; _ }, Some subkind ->
Ok (Sources.ParametricSource { source_name = name; subkind })
| _ -> Error (Format.sprintf "Unsupported taint source `%s`" name)
let parse_sink ~allowed ?subkind name =
let create = function
| "LocalReturn" -> Ok Sinks.LocalReturn
| update when String.is_prefix update ~prefix:"ParameterUpdate" ->
let index = String.chop_prefix_exn update ~prefix:"ParameterUpdate" in
Ok (ParameterUpdate (Int.of_string index))
| name -> Error (Format.sprintf "Unsupported taint sink `%s`" name)
in
match
List.find allowed ~f:(fun { name = sink_name; _ } -> String.equal sink_name name), subkind
with
(* In order to support parsing parametric sources and sinks for rules, we allow matching
parametric source kinds with no subkind. *)
| Some _, None -> Ok (Sinks.NamedSink name)
| Some { kind = Parametric; _ }, Some subkind ->
Ok (Sinks.ParametricSink { sink_name = name; subkind })
| _ -> create name

However, it doesn't display line number and column number.

@onionymous So as you specified yesterday, this issue can be changed to tweak the error codes and display line number and column number with taint.config errors? Just confirming before I proceed further :)

@onionymous
Copy link
Collaborator Author

Yup, as discussed internally with @0xedward and @arthaud, we want to do this on the OCaml side. We already have an existing taint.config parser and does some validation, so this task will be pretty much scoped to having more descriptive error messages on:

  1. duplicate warning code or duplicate source/sink names
  2. when there is a regex error in an implicit string source/sink

Thankfully, since OCaml already uses re2, this means we don't have to deal with packaging an external Python dependency for it - just compiling the regex in OCaml should perform the validation :)

We want this to display useful error messages so that it can be used for integration with e.g. the VSCode plugin in the future, so we probably need information like the line and column number. Right now pyre validate-models actually does a pretty good job for this, but the messages aren't descriptive enough:

e.g. Duplicate source:

λ  distillery  (33b689571|remote/master) pyre validate-models
ƛ Using unstable client. If you encounter issues, you can use the stable client via `export PYRE_CLIENT_RELEASE="stable"`.
ƛ Exception occured during model validation: Invalid payload for model validation: `{'error': '(Failure "Duplicate entry for source: `IP`")'}`.

Malformed regex:

ƛ Exception occured during model validation: Invalid payload for model validation: `{'error': '("Taint__TaintConfiguration.MalformedConfiguration(\\"/data/users/sym/instagram/instagram-server/distillery/stubs/taint/core_privacy_security/open_source/taint.config\\", \\"Line 647, bytes 5-38:\\\\nInvalid token \']\\\\n  },\\\\n  \\\\\\"implicit_sinks\\\\\\": {\\\\n    \'\\")")'}`.

Also going to paste Maxime's internal comment here:

If we are going to extend pyre validate-models there is a small caveat though: I think it requires type checking the whole project. That would make it pretty slow for the VSCode Integration.
We could make this a separate command if needed. We could also check if pyre validate-models could work with an incremental type check (I don't know what it does right now).

We can also think about adding a pyre validate-config command, which skips the typechecking part to make it faster for something like VSCode integration.

@onionymous onionymous changed the title [Fall 2021] Step 3: Add a validator for taint.config files [Fall 2021] Step 3: Add a pyre validate-taint-config command and make errors in taintConfiguration.ml typed Nov 1, 2021
@abishekvashok
Copy link

abishekvashok commented May 5, 2023

After facebook#732 here's a plan of action:

facebook-github-bot pushed a commit to facebook/pyre-check that referenced this issue May 26, 2023
Summary:
**Pre-submission checklist**
- [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install`
- [x] `pre-commit run`

Implements a json parsing module inside jsonParsing.ml that adds support for line and column numbers based on Jsonm. Removes all dependencies of taint configuration parsing on YoJSON which doesn't have any support for positions.

Since, Jsonm is pretty bare bone, implemented some utility functions inside a Util module that does pretty much what YoJSON's utility functions did.

There are minor changes in taintConfiguration to accomodate for the change.

This is the base part of changes that seek to remove dependency completely on YoJSON and switch to JSONM because YoJSON doesn't support line numbers in any way which we require for descriptive error messages.

Modifies and updates confirmationTest. Since we use a proper json parser, the json files for testing have been updated to use the proper json format as well.

Pull Request resolved: #734

Test Plan:
- make
- make test ( only failing test is due to the internal taint integration files -> you know why (: )
<details>

<summary>See test results</summary>

```
../scripts/generate-version-number.sh: line 16: greadlink: command not found
Build info: Darwin x86_64 @ Thu May 11 2023 (development build)
../scripts/generate-version-number.sh: line 30: hg: command not found
Git commit: ced2d4c
dune build install -j auto --profile dev
File "dune", line 379, characters 2-43:
379 |   pyrelib.internalTaintIntegrationTestFiles
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: Library "pyrelib.internalTaintIntegrationTestFiles" not found.
-> required by _build/default/decoratorsTest.exe
-> required by alias runtest in dune:359
................
Ran: 16 tests in: 0.67 seconds.
OK
............
Ran: 12 tests in: 0.42 seconds.
OK
................
Ran: 16 tests in: 0.38 seconds.
OK
.........................................
Ran: 41 tests in: 0.57 seconds.
OK
.........................................
Ran: 41 tests in: 0.74 seconds.
OK
...............................
Ran: 31 tests in: 0.45 seconds.
OK
................................
Ran: 32 tests in: 0.56 seconds.
OK
............
Ran: 12 tests in: 0.32 seconds.
OK
...............................
Ran: 31 tests in: 0.46 seconds.
OK
................
Ran: 16 tests in: 0.35 seconds.
OK
............
Ran: 12 tests in: 0.41 seconds.
OK
.
Ran: 1 tests in: 0.61 seconds.
OK
....
Ran: 4 tests in: 3.25 seconds.
OK
...............
Ran: 15 tests in: 4.43 seconds.
OK
...............
Ran: 15 tests in: 4.60 seconds.
OK
....
Ran: 4 tests in: 0.16 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 1.97 seconds.
OK
.
Ran: 1 tests in: 0.96 seconds.
OK
.
Ran: 1 tests in: 0.64 seconds.
OK
.
Ran: 1 tests in: 1.44 seconds.
OK
..
Ran: 2 tests in: 0.93 seconds.
OK
.
Ran: 1 tests in: 0.47 seconds.
OK
..........
Ran: 10 tests in: 8.32 seconds.
OK
.
Ran: 1 tests in: 0.12 seconds.
OK
..............
Ran: 14 tests in: 0.24 seconds.
OK
.
Ran: 1 tests in: 0.12 seconds.
OK
...
Ran: 3 tests in: 1.19 seconds.
OK
.
Ran: 1 tests in: 0.11 seconds.
OK
.
Ran: 1 tests in: 0.12 seconds.
OK
...
Ran: 3 tests in: 0.14 seconds.
OK
.
Ran: 1 tests in: 0.12 seconds.
OK
.........
Ran: 9 tests in: 0.21 seconds.
OK
..
Ran: 2 tests in: 0.15 seconds.
OK
....
Ran: 4 tests in: 0.20 seconds.
OK
..
Ran: 2 tests in: 0.14 seconds.
OK
......
Ran: 6 tests in: 0.17 seconds.
OK
..
Ran: 2 tests in: 0.13 seconds.
OK
........
Ran: 8 tests in: 0.18 seconds.
OK
..........
Ran: 10 tests in: 0.22 seconds.
OK
.
Ran: 1 tests in: 0.77 seconds.
OK
.......
Ran: 7 tests in: 0.20 seconds.
OK
..
Ran: 2 tests in: 0.86 seconds.
OK
.........
Ran: 9 tests in: 0.43 seconds.
OK
..
Ran: 2 tests in: 0.45 seconds.
OK
..
Ran: 2 tests in: 0.52 seconds.
OK
.....
Ran: 5 tests in: 0.36 seconds.
OK
.
Ran: 1 tests in: 0.68 seconds.
OK
....
Ran: 4 tests in: 1.61 seconds.
OK
.
Ran: 1 tests in: 5.57 seconds.
OK
........
Ran: 8 tests in: 4.05 seconds.
OK
...
Ran: 3 tests in: 0.72 seconds.
OK
....
Ran: 4 tests in: 0.49 seconds.
OK
..........
Ran: 10 tests in: 0.24 seconds.
OK
..
Ran: 2 tests in: 12.12 seconds.
OK
..
Ran: 2 tests in: 0.17 seconds.
OK
.............
Ran: 13 tests in: 0.34 seconds.
OK
.....
Ran: 5 tests in: 1.38 seconds.
OK
.................
Ran: 17 tests in: 12.13 seconds.
OK
...
Ran: 3 tests in: 0.15 seconds.
OK
................
Ran: 16 tests in: 3.29 seconds.
OK
.......
Ran: 7 tests in: 12.86 seconds.
OK
......
Ran: 6 tests in: 2.09 seconds.
OK
.......
Ran: 7 tests in: 0.20 seconds.
OK
..........
Ran: 10 tests in: 0.24 seconds.
OK
.......
Ran: 7 tests in: 5.43 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
....
Ran: 4 tests in: 0.76 seconds.
OK
.....
Ran: 5 tests in: 0.51 seconds.
OK
..
Ran: 2 tests in: 0.40 seconds.
OK
.
Ran: 1 tests in: 0.29 seconds.
OK
.
Ran: 1 tests in: 0.91 seconds.
OK
...................
Ran: 19 tests in: 8.82 seconds.
OK
...............
Ran: 15 tests in: 4.10 seconds.
OK
.
Ran: 1 tests in: 2.91 seconds.
OK
.....
Ran: 5 tests in: 0.21 seconds.
OK
.........................
Ran: 25 tests in: 8.24 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.....
Ran: 5 tests in: 0.18 seconds.
OK
............
Ran: 12 tests in: 35.36 seconds.
OK
.........................
Ran: 25 tests in: 18.38 seconds.
OK
...............................
Ran: 31 tests in: 1.35 seconds.
OK
.........
Ran: 9 tests in: 0.22 seconds.
OK
..
Ran: 2 tests in: 0.14 seconds.
OK
..................
Ran: 18 tests in: 2.56 seconds.
OK
........
Ran: 8 tests in: 3.67 seconds.
OK
.
Ran: 1 tests in: 0.27 seconds.
OK
.
Ran: 1 tests in: 1.55 seconds.
OK
..
Ran: 2 tests in: 0.47 seconds.
OK
............................................................................
Ran: 76 tests in: 1.22 seconds.
OK
.
Ran: 1 tests in: 0.12 seconds.
OK
.....
Ran: 5 tests in: 0.17 seconds.
OK
....
Ran: 4 tests in: 2.19 seconds.
OK
..
Ran: 2 tests in: 0.58 seconds.
OK
............
Ran: 12 tests in: 17.08 seconds.
OK
.
Ran: 1 tests in: 0.54 seconds.
OK
..
Ran: 2 tests in: 12.42 seconds.
OK
.....
Ran: 5 tests in: 3.67 seconds.
OK
......
Ran: 6 tests in: 2.81 seconds.
OK
.
Ran: 1 tests in: 4.54 seconds.
OK
..
Ran: 2 tests in: 3.56 seconds.
OK
........
Ran: 8 tests in: 10.70 seconds.
OK
......
Ran: 6 tests in: 5.11 seconds.
OK
....
Ran: 4 tests in: 2.13 seconds.
OK
..
Ran: 2 tests in: 3.24 seconds.
OK
...........
Ran: 11 tests in: 16.48 seconds.
OK
.
Ran: 1 tests in: 0.40 seconds.
OK
...............................
Ran: 31 tests in: 23.51 seconds.
OK
...
Ran: 3 tests in: 2.24 seconds.
OK
...
Ran: 3 tests in: 2.62 seconds.
OK
............
Ran: 12 tests in: 8.55 seconds.
OK
..
Ran: 2 tests in: 1.58 seconds.
OK
....
Ran: 4 tests in: 5.12 seconds.
OK
..
Ran: 2 tests in: 2.58 seconds.
OK
....
Ran: 4 tests in: 5.86 seconds.
OK
.
Ran: 1 tests in: 0.48 seconds.
OK
...
Ran: 3 tests in: 4.29 seconds.
OK
.
Ran: 1 tests in: 3.70 seconds.
OK
.
Ran: 1 tests in: 0.52 seconds.
OK
..
Ran: 2 tests in: 3.46 seconds.
OK
....
Ran: 4 tests in: 3.03 seconds.
OK
.
Ran: 1 tests in: 0.69 seconds.
OK
........
Ran: 8 tests in: 3.18 seconds.
OK
...
Ran: 3 tests in: 1.90 seconds.
OK
.
Ran: 1 tests in: 2.57 seconds.
OK
.....
Ran: 5 tests in: 5.53 seconds.
OK
.
Ran: 1 tests in: 2.85 seconds.
OK
..........
Ran: 10 tests in: 10.36 seconds.
OK
..............
Ran: 14 tests in: 8.20 seconds.
OK
.......
Ran: 7 tests in: 9.57 seconds.
OK
........
Ran: 8 tests in: 4.38 seconds.
OK
......
Ran: 6 tests in: 9.28 seconds.
OK
.
Ran: 1 tests in: 0.40 seconds.
OK
..
Ran: 2 tests in: 4.01 seconds.
OK
............
Ran: 12 tests in: 12.23 seconds.
OK
..
Ran: 2 tests in: 3.68 seconds.
OK
...
Ran: 3 tests in: 0.14 seconds.
OK
.
Ran: 1 tests in: 1.48 seconds.
OK
..
Ran: 2 tests in: 0.14 seconds.
OK
.....
Ran: 5 tests in: 0.17 seconds.
OK
..
Ran: 2 tests in: 0.14 seconds.
OK
......
Ran: 6 tests in: 23.52 seconds.
OK
...
Ran: 3 tests in: 0.17 seconds.
OK
....
Ran: 4 tests in: 0.16 seconds.
OK
.............
Ran: 13 tests in: 0.27 seconds.
OK
.........................
Ran: 25 tests in: 53.56 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.................
Ran: 17 tests in: 26.98 seconds.
OK
.....
Ran: 5 tests in: 0.17 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.....
Ran: 5 tests in: 0.25 seconds.
OK
.......
Ran: 7 tests in: 0.21 seconds.
OK
..........
Ran: 10 tests in: 0.26 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
..
Ran: 2 tests in: 0.15 seconds.
OK
...
Ran: 3 tests in: 0.15 seconds.
OK
.......Skipping low-level watchman API test due to exception: ("Server.Watchman.ConnectionError(\"Cannot find watchman exectuable under PATH: /Users/abishekvashok/projects/pyre-check/source/_build/install/default/bin:/Users/abishekvashok/.opam/4.14.0/bin:/Users/abishekvashok/.nvm/versions/node/v19.8.1/bin:/usr/local/bin:/usr/local/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Applications/VMware Fusion.app/Contents/Public:/usr/local/go/bin:/Library/Apple/usr/bin:/Users/abishekvashok/Library/Android/sdk/tools:/Users/abishekvashok/Library/Android/sdk/platform-tools:/usr/local/opt/openjdk/bin:/usr/local/opt/binutils/bin\")")

Ran: 7 tests in: 0.22 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
...
Ran: 3 tests in: 0.17 seconds.
OK
..
Ran: 2 tests in: 0.15 seconds.
OK
...
Ran: 3 tests in: 0.16 seconds.
OK
...............
Ran: 15 tests in: 0.30 seconds.
OK
..
Ran: 2 tests in: 0.25 seconds.
OK
.
Ran: 1 tests in: 1.70 seconds.
OK
...
Ran: 3 tests in: 0.15 seconds.
OK
.......
Ran: 7 tests in: 2.33 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.............................
Ran: 29 tests in: 15.11 seconds.
OK
..
Ran: 2 tests in: 0.83 seconds.
OK
.
Ran: 1 tests in: 3.00 seconds.
OK
..............................
Ran: 30 tests in: 20.82 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 0.02 seconds.
OK
.
Ran: 1 tests in: 0.12 seconds.
OK
.
Ran: 1 tests in: 0.12 seconds.
OK
.
Ran: 1 tests in: 1.22 seconds.
OK
..................................................................................................................
Ran: 114 tests in: 54.85 seconds.
OK
....
Ran: 4 tests in: 0.82 seconds.
OK
.....
Ran: 5 tests in: 18.27 seconds.
OK
.
Ran: 1 tests in: 0.12 seconds.
OK
.......
Ran: 7 tests in: 9.23 seconds.
OK
..
Ran: 2 tests in: 0.16 seconds.
OK
..
Ran: 2 tests in: 2.23 seconds.
OK
..
Ran: 2 tests in: 0.16 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 0.15 seconds.
OK
.....
Ran: 5 tests in: 0.23 seconds.
OK
...............................
Ran: 31 tests in: 52.49 seconds.
OK
.
Ran: 1 tests in: 0.14 seconds.
OK
.
Ran: 1 tests in: 0.15 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
.
Ran: 1 tests in: 0.13 seconds.
OK
......
Ran: 6 tests in: 1.09 seconds.
OK
.............
Ran: 13 tests in: 0.57 seconds.
OK
.......
Ran: 7 tests in: 0.86 seconds.
OK
..........
Ran: 10 tests in: 0.32 seconds.
OK
.[2023-05-15 18:04:47] Dumping a saved state deptable.
Finished SQL Transaction took 0.00s
Lookup of existing rows took 95 us
Wrote 10 new rows
Updated 0 existing rows
Finished closing SQL connection took 0.00s
Writing dependency file with sqlite took 0.01s
.............
Ran: 14 tests in: 7.97 seconds.
OK
........................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
Ran: 7496 tests in: 147.70 seconds.
OK
make: *** [test] Error 1
```

</details>

- Run on newly compiled binary on exercise1 in pysa_tutorial (`python3 -m pyre-check.client.pyre -n analyze --no-verify`):
<img width="1179" alt="Screenshot 2023-05-06 at 1 17 36 PM" src="https://user-images.githubusercontent.com/8947010/236610948-15b870e3-fa0e-411f-abac-ab5a20a10cdb.png">

Fixes part of MLH-Fellowship#82

Low level notes:
- This is the root PR that all other PRs relating to the issue mentioned above can/will/might be stacked on top of.
- The pysa Github actions were failing before this PR as well.

Reviewed By: alexkassil

Differential Revision: D46143354

Pulled By: arthaud

fbshipit-source-id: d9bee5dcc3c7e7e9bfbfff532af6d21dfa2af39f
facebook-github-bot pushed a commit to facebook/pyre-check that referenced this issue Jun 1, 2023
Summary:
**Pre-submission checklist**
- [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install`
- [x] `pre-commit run`

Display positions of where duplicate rule codes appear in
RuleCodeDuplicate taint configuration errors.

Previously rule codes were only checked for uniqueness within a single
taint.config file. This leads to unintended results when multiple rules
share the same code in multiple taint.config files under analysis.
The errors emitted also lacked positioning information which makes it
harder to find where duplicate rule codes occurred.

Adds ability to cross validate rule codes as now like source and sink
uniqueness checks, rule codes are checked after all taint.config files
are parsed. Also Since we now store positioning information for all
parsed taint.config nodes, adds position information of the previous
and current location when a rule code appears.

Since uniqueness is checked after parsing, modifies Rules module to
contain the taint config path and location to be used in error
messages - created a module in jsonParsing.ml for the same.

Other minor changes:
- Update tests to check for the new sort of RuleDuplicate error
- Update tests to conform to the updated rule records

Pull Request resolved: #735

Test Plan:
- Before the changes, ran pysa on `documentaiton/pysa_tutorial/exercise1/`:

<img width="1179" alt="Screenshot 2023-05-06 at 1 17 36 PM" src="https://user-images.githubusercontent.com/8947010/236805900-3d42af02-c06f-4663-8286-d432bc1a74a5.png">

- After the changes with default `taint.config`:

<img width="983" alt="Screenshot 2023-05-27 at 5 28 55 PM" src="https://github.com/facebook/pyre-check/assets/8947010/c86d4267-0151-4a1d-a6d6-e2472ae021c8">

- After the changes with the following `taint.config`:
```json
{
  "sources": [
    {
      "name": "CustomUserControlled",
      "comment": "use to annotate user input"
    }
  ],

  "sinks": [
    {
      "name": "CodeExecution",
      "comment": "use to annotate execution of python code"
    }
  ],

  "features": [],

  "rules": [
    {
      "name": "Possible RCE:",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "User specified data may reach a code execution sink"
    },
    {
      "name": "test-duplicate",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "duplicate"
    }
  ]
}
```

<img width="1039" alt="Screenshot 2023-05-31 at 2 06 36 PM" src="https://github.com/facebook/pyre-check/assets/8947010/e0125754-5859-481b-b372-7796ba9166e5">

- Ran tests with `make test`.

Fixes part of MLH-Fellowship#82

Footnotes:
- Pysa Github CI Action was failing before this PR

Reviewed By: tianhan0

Differential Revision: D46352827

Pulled By: arthaud

fbshipit-source-id: e6e3bc30939990a95cf8bea2071f930aa642d989
facebook-github-bot pushed a commit to facebook/pyre-check that referenced this issue Jun 5, 2023
Summary:
Adds verify_taint_config_only option to the analyze command that just verifies taint.config files and skips the analysis. This can be useful in the future when we want to verify taint config files via the vs code extension without performing analysis or even any other sort of preprocessing.

Modifies the ocaml server and the python client to pass options for the same.

**Pre-submission checklist**
- [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install`
- [x] `pre-commit run`

Pull Request resolved: #740

Test Plan:
- Ran pysa with the default `taint.config` on `documentation/pysa_tutorial/exercise1/`:
<img width="955" alt="Screenshot 2023-05-31 at 3 48 17 PM" src="https://github.com/facebook/pyre-check/assets/8947010/66b9d095-77c5-4be3-872d-f98225609de8">

- Command: `python3 -m pyre-check.client.pyre -n analyze --verify-taint-config-only`

<img width="955" alt="Screenshot 2023-05-31 at 3 41 33 PM" src="https://github.com/facebook/pyre-check/assets/8947010/60244953-79e5-4c62-9089-266be884954e">

- Modify `taint.config` to induce a TaintConfigurationError:
```json
{
  "sources": [
    {
      "name": "CustomUserControlled",
      "comment": "use to annotate user input"
    }
  ],

  "sinks": [
    {
      "name": "CodeExecution",
      "comment": "use to annotate execution of python code"
    }
  ],

  "features": [],

  "rules": [
    {
      "name": "Possible RCE:",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "User specified data may reach a code execution sink"
    },
    {
      "name": "test-duplicate",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "duplicate"
    }
  ]
}
```
Command: `python3 -m pyre-check.client.pyre -n analyze --verify-taint-config-only`

<img width="955" alt="Screenshot 2023-05-31 at 3 42 49 PM" src="https://github.com/facebook/pyre-check/assets/8947010/150de5da-f5f0-415d-9407-808a6feb98bd">

- Github actions. (pysa action was failing before this PR)

Fixes part of MLH-Fellowship#82
Signed-off-by: Abishek V Ashok <abishekvashok@gmail.com>

Reviewed By: tianhan0

Differential Revision: D46437429

Pulled By: arthaud

fbshipit-source-id: e1928d79cfe988b903a9c342b0c23cf335364ddd
facebook-github-bot pushed a commit to facebook/pyre-check that referenced this issue Jul 19, 2023
Summary:
**Pre-submission checklist**
- [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install`
- [x] `pre-commit run`

Display TaintConfigurationError locations when available. The Ocaml binary now returns positions after Github PR: #734 (commit 59d2cf0). Parse and print when the location(s) are available.

Pull Request resolved: #739

Test Plan:
Invocation command in all of the below: `python3 -m pyre-check.client.pyre analyze --no-verify`

- Run pyre check with the following `faulty` taint.config: (modify `documentation/pysa_tutorial/exercise1`)
```json
{
  "sources": [
    {
      "name": "CustomUserControlled",
      "comment": "use to annotate user input"
    }
  ],

  "sinks": [
    {
      "name": "CodeExecution",
      "comment": "use to annotate execution of python code"
    }
  ],

  "features": [],

  "rules": [
    {
      "name": "Possible RCE:",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "User specified data may reach a code execution sink"
    }
    {
      "name": "test-duplicate",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "duplicate"
    }
  ]
}
```
- Before:
<img width="1137" alt="Screenshot 2023-05-31 at 2 45 34 PM" src="https://github.com/facebook/pyre-check/assets/8947010/3b2e24c4-98ab-4927-9838-f91592b504e5">

- After:
<img width="1137" alt="Screenshot 2023-05-31 at 2 36 49 PM" src="https://github.com/facebook/pyre-check/assets/8947010/43f6aaf1-5d0a-42ee-889c-e59f10e3beef">

- Run with the stock taint.config: (same results before and after)
<img width="1137" alt="Screenshot 2023-05-31 at 2 48 33 PM" src="https://github.com/facebook/pyre-check/assets/8947010/58aae8d6-0eeb-4cc5-b676-31dcfcdbe826">

- `tox -e py`

- Github Actions (pysa action was failing before this PR and is failing due to an unrelated issue - possibly outdated opam cache?)

Fixes part of MLH-Fellowship#82
Signed-off-by: Abishek V Ashok <abishekvashok@gmail.com>

Reviewed By: saputkin

Differential Revision: D47589562

Pulled By: arthaud

fbshipit-source-id: d17127a38096a1f95ade8648f14a853fd7ab0055
facebook-github-bot pushed a commit to facebook/pyre-check that referenced this issue Jul 19, 2023
Summary:
**Pre-submission checklist**
- [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install`
- [x] `pre-commit run`

Previously when SourceDuplicate errors occured, we just left a note specifying the duplicate name. Now we can specify the locations where the duplicates occured.

Modifies annotationParser as annotationParser.source_or_sink is where the sink or source is parsed into from taint.config files to contain the location information to be used incase duplicates occur.

Modifies and updates tests to confirm and check the new record field and type of SourceDuplicate error.

Pull Request resolved: #741

Test Plan:
- Post changes with the default `taint.config`, running pysa on `documentation/pysa_tutorial/exercise1`

<img width="941" alt="Screenshot 2023-06-02 at 1 21 52 PM" src="https://github.com/facebook/pyre-check/assets/8947010/2dcdae0e-672c-4b95-a888-e73a829a7493">

- Modify `taint.config` to:
```
{
  "sources": [
    {
      "name": "CustomUserControlled",
      "comment": "use to annotate user input"
    },
    {
      "name": "CustomUserControlled",
      "comment": "duplicate for testing"
    }
  ],

  "sinks": [
    {
      "name": "CodeExecution",
      "comment": "use to annotate execution of python code"
    }
  ],

  "features": [],

  "rules": [
    {
      "name": "Possible RCE:",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "User specified data may reach a code execution sink"
    }
  ]
}
```

Before this PR:
<img width="941" alt="Screenshot 2023-06-02 at 1 27 04 PM" src="https://github.com/facebook/pyre-check/assets/8947010/7ad937d4-e4b3-4acf-8fc3-e0646aa39a7d">

After this PR:
<img width="941" alt="Screenshot 2023-06-02 at 1 25 08 PM" src="https://github.com/facebook/pyre-check/assets/8947010/5ef8927d-fa95-44c2-bb82-3ac3532b731f">

- `make test`

Footnotes:
- pysa github action was failing before this PR

Fixes part of: MLH-Fellowship#82
Signed-off-by: Abishek V Ashok <abishekvashok@gmail.com>

Reviewed By: saputkin

Differential Revision: D47589664

Pulled By: arthaud

fbshipit-source-id: 66ccb75f68fb229e033f32a4b7376ed823827330
abishekvashok added a commit to abishekvashok/pyre-check that referenced this issue Jul 20, 2023
Previously when SinkDuplicate errors occured, we just displayed a note.
Now we can display locations where the duplicates occured.

Modifies and updates test as well.

Fixes part of: MLH-Fellowship#82
Signed-off-by: Abishek V Ashok <abishekvashok@gmail.com>
facebook-github-bot pushed a commit to facebook/pyre-check that referenced this issue Jul 20, 2023
Summary:
**Pre-submission checklist**
- [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install`
- [x] `pre-commit run`

Adds new error type that can handle Re2 compilation failed exceptions. Previously, when a regex compilation failed, the exception wasn't caught and the program terminated abnormally.

Catch the exception, and throw a custom TaintConfiguration Error to expose the underlying reason why the compilation failed and exit in a more user-friendly fashion.

Pull Request resolved: #743

Test Plan:
- taint.config used:
```json
{
  "sources": [
    {
      "name": "CustomUserControlled",
      "comment": "use to annotate user input"
    }
  ],
  "implicit_sources": {
     "literal_strings": [
       {
         "regexp": "\\d{1Z,3}(\\.\\d{1,3}+",
         "kind": "CustomUserControlled",
         "description": "String that looks like an IP address."
       }
     ]
  },
  "sinks": [
    {
      "name": "CodeExecution",
      "comment": "use to annotate execution of python code"
    }
  ],

  "features": [],

  "rules": [
    {
      "name": "Possible RCE:",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "User specified data may reach a code execution sink"
    }
  ]
}
```

Before:
<img width="990" alt="before" src="https://github.com/facebook/pyre-check/assets/8947010/2d98a7e3-7650-439b-84e5-e300f758bb47">

After:
<img width="990" alt="after" src="https://github.com/facebook/pyre-check/assets/8947010/5aa82960-eae0-4681-b7e8-949079448042">

Fixes part of: MLH-Fellowship#82
Signed-off-by: Abishek V Ashok <abishekvashok@gmail.com>

- `make test`

Reviewed By: saputkin

Differential Revision: D47589737

Pulled By: arthaud

fbshipit-source-id: eb173e8f20f0aec080abc62ea5c3c2ab21d442e5
facebook-github-bot pushed a commit to facebook/pyre-check that referenced this issue Jul 24, 2023
Summary:
**Pre-submission checklist**
- [x] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running `pip install -r requirements-dev.txt && pre-commit install`
- [x] `pre-commit run`

Previously when SinkDuplicate errors occured, we just displayed a note. Now we can display locations where the duplicates occured.

Modifies and updates test as well.

Pull Request resolved: #756

Test Plan:
- Post changes with the default `taint.config`, running pysa on `documentation/pysa_tutorial/exercise1`

<img width="878" alt="Screenshot 2023-07-20 at 11 46 33 AM" src="https://github.com/facebook/pyre-check/assets/8947010/7edafa01-19b1-4544-94e2-d5f1195c6e41">

- Modify `taint.config` to:

```json
{
  "sources": [
    {
      "name": "CustomUserControlled",
      "comment": "use to annotate user input"
    }
  ],

  "sinks": [
    {
      "name": "CodeExecution",
      "comment": "use to annotate execution of python code"
    },
    {
      "name": "CodeExecution",
      "comment": "duplicate for testing"
    }
  ],

  "features": [],

  "rules": [
    {
      "name": "Possible RCE:",
      "code": 5001,
      "sources": [ "CustomUserControlled" ],
      "sinks": [ "CodeExecution" ],
      "message_format": "User specified data may reach a code execution sink"
    }
  ]
}
```

Before this PR:

<img width="878" alt="Screenshot 2023-07-20 at 11 53 32 AM" src="https://github.com/facebook/pyre-check/assets/8947010/2d69be60-bb9d-4806-a3ae-9d5a83e48f8a">

After this PR:

<img width="878" alt="Screenshot 2023-07-20 at 11 47 49 AM" src="https://github.com/facebook/pyre-check/assets/8947010/c269de27-0351-425d-8912-8e9360884ca9">

- `make test`

Fixes part of: MLH-Fellowship#82
Signed-off-by: Abishek V Ashok <abishekvashok@gmail.com>

Reviewed By: arthaud

Differential Revision: D47717462

Pulled By: r0rshark

fbshipit-source-id: c866752d8ef6f39a3f1ba0346af15cae706884c6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fall 2021 Issues related to the Pysa project for MLH Fellowship cohort of Fall 2021 step 3
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants