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

Clarify code outside build script #63

Open
david-a-wheeler opened this issue Nov 8, 2024 · 6 comments
Open

Clarify code outside build script #63

david-a-wheeler opened this issue Nov 8, 2024 · 6 comments
Assignees

Comments

@david-a-wheeler
Copy link
Contributor

This criterion is not clear:

  - id: OSPS-05
    maturity_level: 1
    category: Build & Release
    criteria: |
      The project's build and release pipelines
      MUST NOT execute arbitrary code that is
      input from outside of the build script.
    objective: |
      Reduce the risk of code injection or other
      security vulnerabilities in the project's
      build and release processes by restricting
      the execution of external code.
    implementation: |
      Ensure that the project's build and release
      pipelines do not execute arbitrary code
      provided from external sources.

On first reading, it sounded like many common practices
are forbidden:

  • pipe-to-shell like curl https://STUFF | sh appears forbidden.
    I'm no fan of pipe-to-shell, as it carries big risks if the
    server is compromised, but it's so widespread that it can't be
    a level 1 requirement. For example, the recommended way to
    install Rust is pipe-to-shell.
  • Downloading components & dependencies to run the
    build script appears forbidden - they must all be downloaded
    before building. Again, not a bad idea, but way too many
    build systems figure out what's needed, then get it.

It appears what's really meant is countering some specific
workflow attacks like script injection, as detected by Scorecard.
That sounds far more tractable, but the current text doesn't say that.

I think this text needs to be redone to say what was actually intended.

@TheFoxAtWork
Copy link

This one (unknown origin) is related to establishment of trust. Ideally this could be resolved by verifying the signature of the code.

so the expectation is that execution of untrusted code input from outside the build script is verified as authentic from the expected source. Even if the build doesnt cut a release, the inclusion of untrusted and unverified code creates an unknown risk.

@funnelfiasco funnelfiasco self-assigned this Dec 10, 2024
@david-a-wheeler
Copy link
Contributor Author

Ben Cotton will try something.

One alternative:
The project’s build process leading to release MUST NOT execute untrusted code.

david-a-wheeler added a commit to david-a-wheeler/security-baseline that referenced this issue Dec 10, 2024
The original requirement is unclear. The discussion
on 2024-12-10 tried to clarify what was intended.
This is one attempt to clarify things.

Signed-off-by: David A. Wheeler <[email protected]>
@david-a-wheeler
Copy link
Contributor Author

See #103 for my attempt to clarify things.

@eddie-knight
Copy link
Contributor

Couldn't find it on the call earlier, but here is the background for this:

ref #103 #104

@evankanderson
Copy link

As written, this also seems to prohibit bugs or mistaken usage of tooling (for example, not pinning a GitHub action to a specific git commit -- the upstream action repo could move a tag to introduce "code that is input from outside the build script"). While that's noble, it makes it hard to figure out what's fair game for automation and project authors to check.

Can we scope this down to specific workflow execution steps? A few axes:

  1. Right now, this says "build and release", but the reference actually is discussing script injections via GitHub events like issues and pull requests, which may not be related to builds or releases (for example, pull_request_target for labelling issues).
  2. The linked GitHub examples are GitHub specific and drawn from published best practices, but the control is written generally to include other systems and covers both best practices and project-specific vulnerabilities.
  3. The rule as written suggests transitive responsibility for these pipelines -- if I call a composite action which executes an erlang application, I'm expected (MUST NOT) to have audited that tool for external code execution. Can we limit this to direct commands and dependencies?

Should this say something like:

Project automation which can be triggered by outside contributors MUST sanitize user-controlled inputs

@evankanderson
Copy link

evankanderson commented Dec 18, 2024

Note that my statement is different from the original in that it doesn't cover validating downloaded or cached resources at maturity level 1. We'd need a different rule for that, possibly at a different maturity level.

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

No branches or pull requests

5 participants