Skip to content

Add documentation of security implications #140

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

Merged
merged 8 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# stub_uploader
# Typeshed Stub Uploader

[![Build status](https://github.com/typeshed-internal/stub_uploader/actions/workflows/check_scripts.yml/badge.svg)](https://github.com/typeshed-internal/stub_uploader/actions/workflows/check_scripts.yml)
[![Upload status](https://github.com/typeshed-internal/stub_uploader/actions/workflows/update_stubs.yml/badge.svg)](https://github.com/typeshed-internal/stub_uploader/actions/workflows/update_stubs.yml)
Expand Down Expand Up @@ -60,3 +60,7 @@ and entering `.*` will select all packages.
Note that this action also sorts packages in the dependency order, but it doesn't update the typeshed commit. It may update
[list of uploaded packages](https://github.com/typeshed-internal/stub_uploader/blob/main/data/uploaded_packages.txt)
if this is the first time the package is uploaded.

## Security Implications

Please see the [SECURITY.md](./SECURITY.md) document for more information.
34 changes: 34 additions & 0 deletions SECURITY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Security Implication of the Typeshed Stub Uploader

Security for the stub uploader is of the highest importance. If the stub
uploader gets compromised, an attacker could upload manipulated stub
packages to gain full access to developer machines or even production hosts.
Considering the high trust, quick turnaround, and automated installation of
stub packages, this could have significant security implications.

## Maintainers

TBD

## Typeshed Data

To ensure that a compromised typeshed repository can't lead to copromised
stub packages, all typeshed data is verified by the stub uploader before
building packages. The stub uploader ensures that only stub and metadata
files are added to the stub packages. This also means that no code from the
typeshed repository must be executed while building packages, and no
modules must be imported.

## Dependencies

Another possible attack vector are dependencies of stub packages.
A compromised dependency can have a similar effect to when a stub package
gets compromised directly. Therefore, only certain dependencies are
allowed:

* Dependencies on other stub packages created by typeshed.
* Dependencies on packages the upstream package depends on – even recursively.
Since it's likely that a stub package gets installed alongside the
upstream package, this does not introduce an additional security liability.
* Dependencies that are explicitly allowlisted in the stub uploader. These
dependencies are vetted to be from a trusted source.
Copy link

Choose a reason for hiding this comment

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

It would be nice to expand upon this and actually describe and explain the possible attack in #61 (comment). This kind of hints at how the attack goes, but IMO does not describe it in enough detail. It is the biggest security issue in stub_uploader I'm aware of, and typeshed maintainers need to be aware of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to hold off documenting this here at the moment. I kept this section quite vague for the moment, mostly because I think we should resolve and implement #90 first and can then discuss the measures and reasons in this document.

Copy link

@Akuli Akuli Jul 22, 2024

Choose a reason for hiding this comment

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

Maybe:

Suggested change
dependencies are vetted to be from a trusted source.
dependencies are vetted to be from a trusted source.
Having one global allowlist is not ideal, because it allows somewhat obscure
dependencies to be added to popular stub packages (their security matters
the most). This may be changed in the near future.