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

🐛Binary-Artifacts: no longer fall back to file extensions #1279

Closed
wants to merge 1 commit into from
Closed

🐛Binary-Artifacts: no longer fall back to file extensions #1279

wants to merge 1 commit into from

Conversation

evverx
Copy link
Contributor

@evverx evverx commented Nov 16, 2021

Looks like isArchive is supposed to cover all the filetypes
the Binary-Artifacts check is interested in:
https://github.com/h2non/filetype#archive. If types like dll
aren't supported they probably should be added to the matchers
library (by the way there is an in-flight PR trying to
achieve exactly that: h2non/filetype#103)

With this patch applied scorecard no longer complains about
systemd:

    {
      "details": null,
      "score": 10,
      "reason": "no binaries found in the repo",
      "name": "Binary-Artifacts",
      "documentation": {
        "url": "https://github.com/ossf/scorecard/blob/63e3b92466f0403159c02f1ccedd43f9400e8b26/docs/checks.md#binary-artifacts",
        "short": "Determines if the project has generated executable (binary) artifacts in the source repository."
      }

and still can successfuly flag repositories with suspicious
elf files for example

./scorecard --verbosity Debug  --show-details --format json  --repo=https://github.com/evverx/binary-artifacts-test --checks Binary-Artifacts | jq | tee OUT
{
  "repo": {
    "name": "github.com/evverx/binary-artifacts-test",
    "commit": "11cbc0fddac398f346639b0552061f859e506b02"
  },
...
  "score": 0.0,
  "checks": [
    {
      "details": [
        "Warn: binary detected: date",
        "Warn: binary detected: nothing-to-see-here-hehe.txt"
      ],
      "score": 0,
      "reason": "binaries present in source code",
      "name": "Binary-Artifacts",
...

Looks like isArchive is supposed to cover all the filetypes
the Binary-Artifacts check is interested in:
https://github.com/h2non/filetype#archive. If types like dll
aren't supported they probably should be added to the matchers
library (by the way there is an in-flight PR trying to
achieve exactly that: h2non/filetype#103)

With this patch applied scorecard no longer complains about
systemd:
```
    {
      "details": null,
      "score": 10,
      "reason": "no binaries found in the repo",
      "name": "Binary-Artifacts",
      "documentation": {
        "url": "https://github.com/ossf/scorecard/blob/63e3b92466f0403159c02f1ccedd43f9400e8b26/docs/checks.md#binary-artifacts",
        "short": "Determines if the project has generated executable (binary) artifacts in the source repository."
      }
```
and still can successfuly flag repositories with suspicious
elf files for example
```
./scorecard --verbosity Debug  --show-details --format json  --repo=https://github.com/evverx/binary-artifacts-test --checks Binary-Artifacts | jq | tee OUT
{
  "repo": {
    "name": "github.com/evverx/binary-artifacts-test",
    "commit": "11cbc0fddac398f346639b0552061f859e506b02"
  },
...
  "score": 0.0,
  "checks": [
    {
      "details": [
        "Warn: binary detected: date",
        "Warn: binary detected: nothing-to-see-here-hehe.txt"
      ],
      "score": 0,
      "reason": "binaries present in source code",
      "name": "Binary-Artifacts",
...
```
return true, nil
} else if _, ok := binaryFileTypes[strings.ReplaceAll(filepath.Ext(path), ".", "")]; ok {
// Falling back to file based extension.
if filetype.IsArchive(content) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the check shouldn't flag pdf files for example I think it would make sense to replace this part with something like

kind, _ := filetype.Archive(content)
if !stopList[kind.Extension] {
...
}

where stopList contains pdf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To get this to work more or less reliably I think some kind of h2non/filetype#73 should be implemented first. I guess I'm back where I started :-)

@evverx evverx marked this pull request as draft November 16, 2021 05:20
@evverx evverx closed this Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant