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

Package name integrity in compartment maps, lavamoat policies #629

Open
dckc opened this issue Mar 22, 2021 · 6 comments
Open

Package name integrity in compartment maps, lavamoat policies #629

dckc opened this issue Mar 22, 2021 · 6 comments
Assignees
Labels
design kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024 lavamoat metamask security blocker Required to make security claims on all expressly supported platforms
Milestone

Comments

@dckc
Copy link
Contributor

dckc commented Mar 22, 2021

At time of writing, the Endo compartment-mapper’s compartmentMapFromNodeModules utility generates a compartment map for a Node.js application as described on disk, as laid out presumably by a Node.js package manager. An alternative approach is to generate a compartment map from the information noted in a package-lock.json or yarn.lock, which have additional metadata.

Lavamoat policies apply to any package with a matching name. There is a potential attack where a dependency declares a dependency with a URL instead of a version range predicate, thereby allowing that package to pose as the named package to the rest of the application. Instead, the Lavamoat policy should apply to any package with a matching resolved name pattern, and the name should be inferred from the dependency entry in the dependee package.json.

This can also be mitigated by having compartmentMapFromNodeModules forbid URL dependency values, but this is probably impractically proscriptive.

cc @erights @kumavis

@kriskowal kriskowal added the security blocker Required to make security claims on all expressly supported platforms label Mar 22, 2021
@kriskowal kriskowal added this to the Endo 1 milestone Mar 22, 2021
@kriskowal kriskowal changed the title Naming Integrity in compartment maps, lavamoat policies Package name integrity in compartment maps, lavamoat policies Mar 23, 2021
@dckc
Copy link
Contributor Author

dckc commented Mar 23, 2021

yarn why has an interesting naming scheme...

=> Found "@agoric/compartment-mapper#ses@0.11.0"
info This module exists because "_project_#@agoric#bundle-source#@agoric#compartment-mapper" depends on it.

@kriskowal
Copy link
Member

See also #619

@kumavis
Copy link
Member

kumavis commented Mar 29, 2021

@dckc theres a few other ways a "non-canonical" (wrt to the npm registry) package name can be used, such as bundledDependencies. I have a poc in my bad-ideas-collection

@kriskowal
Copy link
Member

@kumavis @naugtur Can we consider this resolved with the use of A’a for naming packages in compartment map policies?

@kriskowal kriskowal added the kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024 label Jan 10, 2024
@naugtur
Copy link
Member

naugtur commented Jan 15, 2024

Policies are applied based on the path in the graph created as input to building a compartment map. path field is equivalent to aa (ʻaʻā) if you disregard how some special cases are handled.
I agree we should test against examples of conflicting packages, but I don't think compartment-map is vulnerable to trusting two different packages matching a name with the same policy.

Meanwhile, we're also debating alternatives to current aa canonical names that would not rely on the layout of node_modules on disk. Package name should be distinct by dependency source in all cases but bundled dependencies so we'd only need to find a way to identify those.

@naugtur
Copy link
Member

naugtur commented Jan 16, 2024

I've created a test case specifically for that bundled dependency scenario.
Our malicious package example, eve, now has a dependency named alice, which is a name used elsewhere.
Observations:

We do have a place in the codebase that seems to be affected by this trick though. Not sure where, but the test is failing with The bundler and importer should agree on source map count and it seems like the sourcemap for the attenuator is not being loaded in this case, for which I have no idea how it could be affected by the fixture.

@kriskowal I might need some help figuring this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design kriskowal-review-2024-01 Issues that kriskowal wants to bring to the attention of the team for review as of January, 2024 lavamoat metamask security blocker Required to make security claims on all expressly supported platforms
Projects
None yet
Development

No branches or pull requests

5 participants