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

Harden PPA defined type #1052

Merged
merged 2 commits into from
Aug 18, 2022
Merged

Harden PPA defined type #1052

merged 2 commits into from
Aug 18, 2022

Conversation

chelnak
Copy link

@chelnak chelnak commented Aug 11, 2022

Prior to this PR there was a possibility that malformed strings could be passed as the resources name.
This could lead to unsafe executions on a remote system.

This was also a possibility for the options parameter as it was constrained to a string.

In addition, commands were not properly broken out in to arrays of arguments when passed to the exec resource.

This PR fixes the above by adding validation to the resource name ensuring that the given ppa name conforms to expectation.

Also, commands are now broken down in to arrays of arguments appropriately. This ensures safer execution on the remote system.

Given that the options parameter, passed as a raw string, could lead to unsafe code execution it was reasonable to change the accepted type to an Optional[Array[String]].

This means that an array of options can now be passed to the exec resource inside the original command.

@puppet-community-rangefinder
Copy link

apt is a class

Breaking changes to this file WILL impact these 245 modules (exact match):
Breaking changes to this file MAY impact these 47 modules (near match):

apt::params is a class

that may have no external impact to Forge modules.

apt::ppa is a type

Breaking changes to this file WILL impact these 68 modules (exact match):
Breaking changes to this file MAY impact these 10 modules (near match):

This module is declared in 234 of 579 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@chelnak chelnak changed the title Harden PPA Harden PPA defined type Aug 12, 2022
@chelnak chelnak marked this pull request as ready for review August 12, 2022 12:26
@chelnak chelnak requested a review from a team as a code owner August 12, 2022 12:26
LivingInSyn
LivingInSyn previously approved these changes Aug 15, 2022
lib/facter/apt_list_sources.rb Outdated Show resolved Hide resolved
@chelnak chelnak force-pushed the maint-harden_ppa branch 7 times, most recently from c1eb559 to 79c9634 Compare August 17, 2022 10:32
manifests/ppa.pp Outdated Show resolved Hide resolved
LivingInSyn
LivingInSyn previously approved these changes Aug 17, 2022
binford2k
binford2k previously approved these changes Aug 17, 2022
Copy link

@binford2k binford2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one small and optional suggestion

manifests/ppa.pp Outdated Show resolved Hide resolved
spec/defines/ppa_spec.rb Show resolved Hide resolved
manifests/ppa.pp Show resolved Hide resolved
Prior to this commit there was a possibility that malformed strings
could be passed as the resources name. This could lead to unsafe
executions on a remote system.

This was also a possibility for the options parameter as it was
constrained to a string.

In addition, commands were not properly broken out in to arrays of
arguments when passed to the exec resource.

This commit fixes the above by adding validation to the resource name
ensuring that the given ppa name conforms to expectation. Also, commands
are now broken down in to arrays of arguments appropriately. This ensures
safer execution on the remote system.

Given that the options parameter, passed as a raw string, could lead to
unsafe code execution it was reasonable to change the accepted type to
an `Optional[Array[String]]. This means that an array of options can now
be passed to the exec resource inside the original command.
This commit adds spec tests that validate the changes made in
the previous commit.
@chelnak chelnak dismissed stale reviews from binford2k and LivingInSyn via 3a90419 August 18, 2022 07:28
@pmcmaw pmcmaw merged commit 83082c0 into main Aug 18, 2022
@pmcmaw pmcmaw deleted the maint-harden_ppa branch August 18, 2022 08:23
LukasAud added a commit that referenced this pull request Jan 17, 2023
Prior to this commit, one of our updates (#1052)
implemented a regex validation for ppa packages that were to be
installed. However, this validation did not account for resource
names that were dotted.

This commit aims to fix this bug in our validation process so that it
works as intended.
LukasAud added a commit that referenced this pull request Jan 18, 2023
Prior to this commit, one of our updates (#1052)
implemented a regex validation for ppa packages that were to be
installed. However, this validation did not account for resource
names that were dotted.

This commit aims to fix this bug in our validation process so that it
works as intended.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants