endo icon indicating copy to clipboard operation
endo copied to clipboard

Package name integrity in compartment maps, lavamoat policies

Open dckc opened this issue 4 years ago • 6 comments

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

dckc avatar Mar 22 '21 21:03 dckc

yarn why has an interesting naming scheme...

=> Found "@agoric/compartment-mapper#[email protected]"
info This module exists because "_project_#@agoric#bundle-source#@agoric#compartment-mapper" depends on it.

dckc avatar Mar 23 '21 23:03 dckc

See also #619

kriskowal avatar Mar 26 '21 00:03 kriskowal

@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

kumavis avatar Mar 29 '21 05:03 kumavis

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

kriskowal avatar Jan 10 '24 01:01 kriskowal

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 avatar Jan 15 '24 12:01 naugtur

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:

  • the way the nested package has to be identified for policy is eve>alice instead of alice
  • it does not get confused for any other alice and permissions aren't granted (as seen in the test result matching the assertion here https://github.com/endojs/endo/pull/1967/files#diff-ddb1847c929bf61b1fd65b8b3c34028d5be8ec2b3d932e98271ab62e94be2c8bR143)

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.

naugtur avatar Jan 16 '24 07:01 naugtur