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

Accept lists of URIs #2

Closed
JulienPalard opened this issue Jun 22, 2020 · 9 comments · Fixed by #49
Closed

Accept lists of URIs #2

JulienPalard opened this issue Jun 22, 2020 · 9 comments · Fixed by #49
Labels
enhancement New feature or request

Comments

@JulienPalard
Copy link

Thanks for opening ChopChop!

Looked at chopchop.yml and though « I'll gladly add some... », but wanted to do it like so:

- uri: ["/db.sql", "/db.sql.gz", "/db.sqlite", "/db.sqlite.gz", "/db.sqlite3", "/db.sqlite3.gz", "/data.sql", "/data.sql.gz", "/users.sql", "/users.sql.gz", "/dump.sql", "/dump.sql.gz", "/mysqldump.sql", "/mysqldump.sql.gz", "/backup.sql", "/backup.sql.gz", "/db.backup", "/db.backup.gz", "/database.sql", "/database.sql.gz", "/db-data.sql", "/db-data.sql.gz", "/db_test.sql", "/db_test.sql.gz", "/db-test.sql", "/db-test.sql.gz"]
  checks:
    - name: Database file
      status_code: 200
      remediation: Delete this file
      description: Verifies a database dump is accessible.
      severity: "High"

(and I bet we could continue for hours adding to this list)

As you imagine, I don't want to copy/paste the name, status_code, remediation, description, and severity 26 times.

Doodling around the idea, it would be great to be able to express those as a « genex », or something similar, something like:

/(db|database|backup|mysqldump|dump|data).(sql|sqlite|sqlite3)(\.gz)?
@JulienPalard JulienPalard changed the title Accept lists as uri Accept lists in uri Jun 22, 2020
@JulienPalard JulienPalard changed the title Accept lists in uri Accept lists of URIs Jun 22, 2020
@faithfulnessalamu
Copy link

Is this a good first issue?

@ghost
Copy link

ghost commented Jun 24, 2020

Thanks for opening ChopChop!

Looked at chopchop.yml and though « I'll gladly add some... », but wanted to do it like so:

- uri: ["/db.sql", "/db.sql.gz", "/db.sqlite", "/db.sqlite.gz", "/db.sqlite3", "/db.sqlite3.gz", "/data.sql", "/data.sql.gz", "/users.sql", "/users.sql.gz", "/dump.sql", "/dump.sql.gz", "/mysqldump.sql", "/mysqldump.sql.gz", "/backup.sql", "/backup.sql.gz", "/db.backup", "/db.backup.gz", "/database.sql", "/database.sql.gz", "/db-data.sql", "/db-data.sql.gz", "/db_test.sql", "/db_test.sql.gz", "/db-test.sql", "/db-test.sql.gz"]
  checks:
    - name: Database file
      status_code: 200
      remediation: Delete this file
      description: Verifies a database dump is accessible.
      severity: "High"

(and I bet we could continue for hours adding to this list)

As you imagine, I don't want to copy/paste the name, status_code, remediation, description, and severity 26 times.

Doodling around the idea, it would be great to be able to express those as a « genex », or something similar, something like:

/(db|database|backup|mysqldump|dump|data).(sql|sqlite|sqlite3)(\.gz)?

Status Code 200 will give you 90% false positives for database backups.
A valid fileheader would eliminate this problem.
Not tested tho:

  - uri: "/backup.sql"
    checks:
      - name: SQL Backup
        match:
          - '{"status"'
        headers:
          - "Content-Type:application/sql"
        remediation: Delete it.
        description: Backupfile of your database. Sensitive informations like admin login could be readable.
        severity: "High"

@isontheline
Copy link
Contributor

Bonjour @JulienPalard, jamais entendu parler de "genex" auparavant 👴, c'est génial !

I agree with Julien, list of URIs is needed in a lot of use cases.
Implementation of "genex" has also been implemented into Golang : https://github.com/alixaxel/genex

@PaulSec could I help to implement this feature inside ChopChop?

@PaulSec
Copy link
Collaborator

PaulSec commented Aug 14, 2020

Definitely! Go for it and open a pull request. I will review it ASAP when I will be back from holidays!

@DloomPlz
Copy link
Collaborator

Same here ! I'll work on multi-threading and refactor of code in the mean time.

@PaulSec PaulSec added the enhancement New feature or request label Sep 25, 2020
@PaulSec
Copy link
Collaborator

PaulSec commented Oct 16, 2020

The problem is known in the library we are using, see: go-yaml/yaml#100

Would you find acceptable that the uri parameter is treated as an array all the time? Eg.

  - uri: "/phpinfo.php"
    checks:
      - name: PHPInfo
        match:
          - 'phpinfo()'
        remediation: Disable phpinfo() in PHP.ini
        description: Checks that the phpinfo() function is accessible
        severity: "Low"
        tested: true

Would become:

  - uri: ["/phpinfo.php"]
    checks:
      - name: PHPInfo
        match:
          - 'phpinfo()'
        remediation: Disable phpinfo() in PHP.ini
        description: Checks that the phpinfo() function is accessible
        severity: "Low"
        tested: true

In this case, we wouldn't have any issue in order to unmarshal the data and we could allow array of uri.

@JulienPalard
Copy link
Author

It would break already written yml, I don't know if there's any though.

An alternative would be to allow for the uri (an optional string) and the uris (an optional list of strings), to keep compatibility and save three chars when there's a single one:

- uri: "/phpinfo.php"

and

- uris: ["/phpinfo.php", "/phpinfo"]

It raises the ambiguity of giving both, ambiguity is bad, but it's probably simple to resolve : hit both uri and uris, and warn.

I bet if genex are implemented they'll have to be in separated field to, to avoid mis-interpreting paths with strange chars as genex.

@PaulSec
Copy link
Collaborator

PaulSec commented Oct 16, 2020

Definitely, you're right.
Ok, let's go for two specific fields and hit both on each plugin.

I will write a pull-request and let you know when it's ready (I guess this week-end at least)

PaulSec added a commit that referenced this issue Oct 16, 2020
`uri` and `uris` can't be specified at the same time.

One example configuration file is as follow:

```yaml
  - uris: ["/db.sql", "/db.sql.gz", "/db.sqlite", "/db.sqlite.gz", "/db.sqlite3", "/db.sqlite3.gz", "/data.sql", "/data.sql.gz", "/users.sql", "/users.sql.gz", "/dump.sql", "/dump.sql.gz", "/mysqldump.sql", "/mysqldump.sql.gz", "/backup.sql", "/backup.sql.gz", "/db.backup", "/db.backup.gz", "/database.sql", "/database.sql.gz", "/db-data.sql", "/db-data.sql.gz", "/db_test.sql", "/db_test.sql.gz", "/db-test.sql", "/db-test.sql.gz"]
    checks:
      - name: Database file
        status_code: 200
        remediation: Delete this file
        description: Verifies a database dump is accessible.
        severity: "High"
```

And we tried it using:

```bash
./gochopchop scan -u http://127.0.0.1:3000 --timeout 1 --csv --csv-file boo.csv -c policy.yml
```

Closes #2
@PaulSec
Copy link
Collaborator

PaulSec commented Oct 16, 2020

Feel free to check out the pull request, it should help you out 🚀

PaulSec added a commit that referenced this issue Oct 23, 2020
`uri` and `uris` can't be specified at the same time.

One example configuration file is as follow:

```yaml
  - uris: ["/db.sql", "/db.sql.gz", "/db.sqlite", "/db.sqlite.gz", "/db.sqlite3", "/db.sqlite3.gz", "/data.sql", "/data.sql.gz", "/users.sql", "/users.sql.gz", "/dump.sql", "/dump.sql.gz", "/mysqldump.sql", "/mysqldump.sql.gz", "/backup.sql", "/backup.sql.gz", "/db.backup", "/db.backup.gz", "/database.sql", "/database.sql.gz", "/db-data.sql", "/db-data.sql.gz", "/db_test.sql", "/db_test.sql.gz", "/db-test.sql", "/db-test.sql.gz"]
    checks:
      - name: Database file
        status_code: 200
        remediation: Delete this file
        description: Verifies a database dump is accessible.
        severity: "High"
```

And we tried it using:

```bash
./gochopchop scan -u http://127.0.0.1:3000 --timeout 1 --csv --csv-file boo.csv -c policy.yml
```

Closes #2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants