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

Security: force update yarn version in Dockerfile #48

Merged
merged 1 commit into from Jan 14, 2020

Conversation

@Beanow
Copy link
Member

Beanow commented Dec 20, 2019

Same changes as sourcecred/sourcecred#1503


Note, unless you used the SourceCred Docker image's bundled
npm or yarn to install your own package.json dependencies,
you were not vulnerable. Otherwise the same risk applies as
in this NPM blog.

You can patch the vulnerability by using the latest Docker image
using docker pull sourcecred/sourcecred as soon as this commit
is included in the latest release.

Commit details

In a recent security issue found in NPM and Yarn, handling
binary file installation has changed. Quoting from there:

The bin script linking libraries in use in npm v6.13.4 were
updated such that, when installing binary entries of top-level
globally installed packages, they will only overwrite existing
binary files if they are currently installed on behalf of the
same package being installed. For example, npm install –global
foo could overwrite /usr/local/bin/foo if and only if
/usr/local/bin/foo is currently a link to a previously installed
version of foo.

In our case, we specifically want this behavior in our Dockerfile.
The node:12 base image comes with an NPM and Yarn version installed.
We're using npm i -g yarn@<version> to upgrade the yarn installation
to a predictable minimum, should we have an older version from the
base image. But since they're from different installation sources,
it causes an error as it would overwrite the yarn binary that wasn't
previously owned by npm install.

Our own package.json or yarn.lock did not appear to have any risk
of exploitation. However since we bundle our image with npm and yarn,
people using our image could in theory use it to install their own
packages. Meaning we should include the fixed npm and yarn versions
to protect users in such a scenario.


Test plan:

# Build with latest node base image.
docker pull node:12
docker build -t widgets .

# Expect: 6.13.4 (or greater)
docker run --rm --entrypoint="" widgets npm -v

# Expect: 1.21.1 (or greater)
docker run --rm --entrypoint="" widgets yarn -v
Note, unless you used the SourceCred Docker image's bundled
npm or yarn to install your own package.json dependencies,
you were not vulnerable. Otherwise the same risk applies as
[in this NPM blog][1].

You can patch the vulnerability by using the latest Docker image
using `docker pull sourcecred/sourcecred` as soon as this commit
is included in the latest release.

## Commit details

In a [recent security issue][1] found in NPM and Yarn, handling
binary file installation has changed. Quoting from there:

> The bin script linking libraries in use in npm v6.13.4 were
> updated such that, when installing binary entries of top-level
> globally installed packages, they will only overwrite existing
> binary files if they are currently installed on behalf of the
> same package being installed.  For example, npm install –global
> foo could overwrite /usr/local/bin/foo if and only if
> /usr/local/bin/foo is currently a link to a previously installed
> version of foo.

In our case, we specifically want this behavior in our Dockerfile.
The node:12 base image comes with an NPM and Yarn version installed.
We're using npm i -g yarn@<version> to upgrade the yarn installation
to a predictable minimum, should we have an older version from the
base image. But since they're from different installation sources,
it causes an error as it would overwrite the yarn binary that wasn't
previously owned by npm install.

Our own package.json or yarn.lock did not appear to have any risk
of exploitation. However since we bundle our image with npm and yarn,
people using our image could in theory use it to install their own
packages. Meaning we should include the fixed npm and yarn versions
to protect users in such a scenario.

[1]: https://blog.npmjs.org/post/189618601100/binary-planting-with-the-npm-cli
@Beanow

This comment has been minimized.

Copy link
Member Author

Beanow commented Jan 14, 2020

The change has a track record with the main repository. So I'm going ahead to include it here.

@Beanow Beanow merged commit 240ef43 into master Jan 14, 2020
2 checks passed
2 checks passed
ci/circleci: publish-1 Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
@Beanow Beanow deleted the pr/docker-image-security branch Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant
You can’t perform that action at this time.