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

Were there recent changes to install.ps1 file? #3

Closed
artisticcheese opened this issue Feb 5, 2021 · 10 comments
Closed

Were there recent changes to install.ps1 file? #3

artisticcheese opened this issue Feb 5, 2021 · 10 comments

Comments

@artisticcheese
Copy link

artisticcheese commented Feb 5, 2021

Please see the issue which was not there a day ago
chocolatey/cChoco#151

┆Issue is synchronized with this GitLab issue by Unito

@gep13
Copy link
Member

gep13 commented Feb 5, 2021

@artisticcheese yes, a new version of the install.ps1 file was released today, which addressed a number of issues which were identified.

Are you seeing a problem with it?

@artisticcheese
Copy link
Author

Yes, it's not throwing terminating issue when it thinks chocolatey is already installed which causing DSC cChocolatey to proceed with creation of environment variables etc thinking installation successfully proceeded when it did not.

@vexx32
Copy link
Member

vexx32 commented Feb 5, 2021

Yeah, currently it just writes a warning. From what I remember, I think @ferventcoder wanted it as a warning rather than an error, but perhaps it's better if this is a terminating error?

@gep13
Copy link
Member

gep13 commented Feb 5, 2021

Adding @vexx32 for visibility.

@artisticcheese
Copy link
Author

As of right now (with current logic) all cChoco DSC module users will have problems since it's different behavior which was before when I assume it was terminating issue or logic itself was different.
The way cChoco works is that it places install.ps1 file into the folder where chocolatey is being installed and current logic considers non-empty folder as a sign that chocolatey is installed which is not

@ferventcoder
Copy link
Member

Sounds like there is an issue with the script - this is covering a security issue where Chocolatey should not install into a pre-created folder (what becomes C:\ProgramData\chocolatey).

@vexx32
Copy link
Member

vexx32 commented Feb 5, 2021

Might it be better for cChoco to place the script either in a temp directory or just add the script after running the install.ps1 to install chocolatey itself, if the script really needs to be in the Chocolatey directory?

@ferventcoder
Copy link
Member

ferventcoder commented Feb 5, 2021

Yeppers, cChoco should place the script elsewhere.

chocolatey/cChoco#151 (comment) is the biggest part of why this change needed to occur. With security changes, we do what we can to minimize impact, but unfortunately it doesn't mean we'll catch everything ahead of time.

The problem lies in that someone could pre-create the folder and drop a hijacking dll in here. Then when Chocolatey installs and puts folders on the SYSTEM path, those hijacking DLLs are still there. Even though it installs and asserts admin privileges to the default location (not to C:\Choco - you are left to handle those when you customize the path), it takes everything existing along for the ride.

So to fix the security issue, if there is an existing folder where Chocolatey was going to install, it just doesn't. Errorring in a terminating way on this is just completely out - that would cause a lot of consternation as compared to what is currently being seen (as in a LOT more folks would be affected).

@EricHripko
Copy link

Also seeing issues (around proxies) with the recent changes in the install script. Couldn't find the source repository for it, so filed an issue on the main repo: https://github.com/chocolatey/choco/issues/2195.

@gep13
Copy link
Member

gep13 commented Feb 10, 2021

@artisticcheese a new version of cChoco has been released which addresses the issue that was raised here.

You can find information about this release here:

https://github.com/chocolatey/cChoco/releases/tag/v2.5.0

I am going to go ahead and close out this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants