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

It's possible to inject malicious packages on non-existant repository URLs #9694

Closed
codekandis opened this issue Feb 11, 2021 · 8 comments
Closed

Comments

@codekandis
Copy link
Contributor

codekandis commented Feb 11, 2021

According to this article
https://medium.com/@alex.birsan/dependency-confusion-4a5d60fec610
and this german short about it
https://www.heise.de/news/Sicherheitsforscher-bricht-ueber-Open-Source-Repositories-bei-PayPal-Co-ein-5051635.html
I did some research and testing.

Assume I have a non-packagist repository in use called "vendor/non-packagist-repo".

{
	"name": "vendor/some-application",
	"minimum-stability": "stable",
	"prefer-stable": true,
	"require": {
		"vendor/non-packagist-repo": "^0"
	},
	"repositories": {
		"vendor/non-packagist-repo": {
			"type": "vcs",
			"url": "file:///some/path/to/the/repository/vendor/non-packagist-repo.git"
		}
	}
}

Even if a repository with the same name exists on packagist composer resolves the given repository URL and pulls that non-packagist repository.

Now assume there is a repository on packagist which contains malicious code. Furtheron assume one has a typo in the repository URL which leads to a non-existing repository

{
	"name": "vendor/some-application",
	"minimum-stability": "stable",
	"prefer-stable": true,
	"require": {
		"vendor/non-packagist-repo": "^0"
	},
	"repositories": {
		"vendor/non-packagist-repo": {
			"type": "vcs",
			"url": "file:///some/path/to/the/repository/vendor/non-existing-repo.git"
		}
	}
}

composer cannot resolve the repository URL but doesn't reject installing the package. Instead it pulls the malicious packagist repository without any hint or warning.

I expect this to happen

As soon as I explicitely reference a repository by its name in the repository section

"repositories": {
	"vendor/non-packagist-repo": {...}
}

I expect composer to validate the referenced repository data and to reject the repository pull. The explicit reference in fact means `I want you to pull this one! Not another one! This one!"

If no explicit reference is given

"repositories": [
	{
		"type": "vcs",
		"url": "file:///some/path/to/the/repository/vendor/non-packagist-repo.git"
	}
]

composer is free to lookup on further resources. Because I want you to look up first at this URL. But you're free to use another source.

Composer version

2.0.9 2021-01-27 16:09:27

@stof
Copy link
Contributor

stof commented Feb 11, 2021

vendor/non-packagist-repo is not a package name in your composer.json repositories. It is a repository name. Repositories can contain many packages (for instance the default repository packagist.org contains thousands of packages).

@Seldaek
Copy link
Member

Seldaek commented Feb 11, 2021

The repository "name" like you have in "vendor/non-packagist-repo" has no meaning, this is something you made up, but to Composer this is just a string/identifier and does not mean it's a package name that will be found in there. So I don't see how your expectation could possibly be fulfilled.

Note that we did publish a blog post highlighting how Composer handles this and possible ways to protect yourself further: https://blog.packagist.com/preventing-dependency-hijacking/

Will close here because I don't see a reasonable way to achieve what you are asking. Using inline VCS repos is anyway a bad practice IMO except in special temporary situations, and using proper composer repositories like Private Packagist makes more sense.

@Seldaek Seldaek closed this as completed Feb 11, 2021
@codekandis
Copy link
Contributor Author

codekandis commented Feb 11, 2021

So I'm taking your mentions in account and making some changes in the expectations.

Using inline VCS repos is anyway a bad practice IMO except in special temporary situations, and using proper composer repositories like Private Packagist makes more sense.

That suggestion does not respect the needs of privacy and data protection. We have packages we cannot store on a public service like GitHub / Packagist. To have them in a private account doesn't change the requirements. It'll still violates our requirements.

We still want to be able to use composer. So we're using inline VCS repositories. And this must be possible without creating a dummy vendor on packagist for nothing else but preventing a dependency hack.

As of Composer 2.0 custom repositories are canonical by default.

If a repository URL can be resolved and no package is found in there - ok. This is a go. But if a repository URL cannot be resolved there must be assumed this is unwanted. The usage of the repository has a meaning because I want my packages used from that repository in the first place, if it contains that packages. If the repository cannot be resolved for whatever reason like site down or even a typo composer must fail.

Consider a CD process where the application will be deployed and no exception occurs while a repository is down and a malicious package has been pulled. In a CD process no one checks the logs until something excepts.

@stof
Copy link
Contributor

stof commented Feb 11, 2021

That suggestion does not respect the needs of privacy and data protection. We have packages we cannot store on a public service like GitHub / Packagist. To have them in a private account doesn't change the requirements. It'll still violate our requirements.

you can still use Satis or a self-hosted private packagist to have a private composer repository hosted in your own infrastructure if you cannot use cloud-based solutions even when dealing with private repositories.

Consider a CD process where the application will be deployed and no exception occurs while a repository is down and a malicious package has been pulled. In a CD process no one checks the logs until something excepts.

A CD process should never be running composer update but composer install with an existing lock file. In such case, it will never read the repositories anyway, but only the lock file.

@codekandis
Copy link
Contributor Author

codekandis commented Feb 11, 2021

A CD process should never be running composer update but composer install with an existing lock file. In such case, it will never read the repositories anyway, but only the lock file.

That's not thought through.
What if the repository wasn't found during the development process? Currently I don't get any message about it. So I commit a faulty lock file with a package resolved to a public vendor? Do I have to check my lock-files on every update in the dev process?

@Seldaek
Copy link
Member

Seldaek commented Feb 11, 2021

We still want to be able to use composer. So we're using inline VCS repositories. And this must be possible without creating a dummy vendor on packagist for nothing else but preventing a dependency hack.

See point six in https://blog.packagist.com/preventing-dependency-hijacking/

Do I have to check my lock-files on every update in the dev process?

Yes you should review what you commit to your repo IMO. That's point 5 in the blog post above.


Anyway. I had a look at what you mentioned and did indeed discover an issue in that proc_open (and by extension symfony/process and our own ProcessExecutor class) seems to silently ignore when $cwd does not exist. So in your git repo's case, it ends up running as if you had a "url": "." in the repo definition, so it will return package versions for your project's git repo, and thus it works.

So it's really a weird bug leading it to silently continue when given an invalid path. This fact wasn't clear to me in your initial report, and I absolutely agree it should fail if a repo URL is not valid.

See #9695

@Seldaek
Copy link
Member

Seldaek commented Feb 11, 2021

@codekandis you can try again after composer self-update --snapshot - it should resolve it.

@codekandis
Copy link
Contributor Author

codekandis commented Feb 11, 2021

So it's really a weird bug leading it to silently continue when given an invalid path. This fact wasn't clear to me in your initial report, and I absolutely agree it should fail if a repo URL is not valid.

See #9695

@codekandis you can try again after composer self-update --snapshot - it should resolve it.

Thank you very much. It does resolve it in e7f6dd2

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

3 participants