Harden apt-mark defined type #1051
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apt_mark is a typethat may have no external impact to Forge modules. This module is declared in 234 of 579 indexed public
|
chelnak
force-pushed
the
maint-move_apt_mark_to_provider
branch
5 times, most recently
from
75cef27 to
a6a9562
Compare
Prior to this commit the title parameter of this defined type was not properly validated. This means that it could have been possible to use a resource title outside of the normal bounds of a package name. Additionally the `onlyif` and `command` parameter values were interpolated strings meaning that it may have been possible to execute unsafe code on the remote system. This commit fixes the above issues by adding a regex to check that the resource title is a valid apt package name and also breaks out the `onlyif` and `command` parameter values in to arrays of args ensuring that the commands executed in a safe manor on the remote system. The exception in this commit is the `unless_cmd`. This has not been broken out in to an array of args due to the requirement of the command. This is a reasonable trade of however due to the fact that action is created from known enum values and title would be pre-validated. This is also explained in mark.pp:20.
chelnak
force-pushed
the
maint-move_apt_mark_to_provider
branch
from
a6a9562 to
35b491b
Compare
This commit adds additional spec tests for mark.pp. The tests validate the new resource name requirements introduced in the previous commit.
chelnak
force-pushed
the
maint-move_apt_mark_to_provider
branch
from
35b491b to
79bec3d
Compare
LivingInSyn
approved these changes
Aug 15, 2022
binford2k
approved these changes
Aug 16, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this PR the title parameter of this defined type was not properly validated.
This means that it could have been possible to use a resource title outside of the normal bounds of a package name.
Additionally the
onlyifandcommandparameter values were interpolated strings meaning that it may have been possible to execute unsafe code on the remote system.This PR fixes the above issues by adding a regex to check that the resource title is a valid apt package name and also breaks out the
onlyifandcommandparameter values in to arrays of args.By doing this we are ensuring that the commands executed in a safe manor on the remote system.
The exception to this is the
unless_cmd.This has not been broken out in to an array of args due to the requirement of the command (explained in more detail in mark.pp:20).
Using an interpolated string here is a reasonable trade off however, due to the fact that action is created from known enum values and title would be pre-validated by the regular expression.