Skip to content

Commit eed10ea

Browse files
committed
Harden apt-mark defined type
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.
1 parent 4b12e7b commit eed10ea

File tree

2 files changed

+28
-10
lines changed

2 files changed

+28
-10
lines changed

examples/mark.pp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
apt::mark { 'vim':
2+
setting => 'auto',
3+
}

manifests/mark.pp

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,31 @@
88
define apt::mark (
99
Enum['auto','manual','hold','unhold'] $setting,
1010
) {
11-
case $setting {
12-
'unhold': {
13-
$unless_cmd = undef
14-
}
15-
default: {
16-
$unless_cmd = "/usr/bin/apt-mark show${setting} ${title} | /bin/fgrep -qs ${title}"
17-
}
11+
if $title !~ /^[a-zA-Z0-9\-_]+$/ {
12+
fail("Invalid package name: ${title}")
1813
}
19-
exec { "/usr/bin/apt-mark ${setting} ${title}":
20-
onlyif => "/usr/bin/dpkg -l ${title}",
21-
unless => $unless_cmd,
14+
15+
if $setting == 'unhold' {
16+
$unless_cmd = undef
17+
} else {
18+
$action = "show${setting}"
19+
20+
# It would be ideal if we could break out this command in to an array of args, similar
21+
# to $onlyif_cmd and $command. However, in this case it wouldn't work as expected due
22+
# to the inclusion of a pipe character.
23+
# When passed to the exec function, the posix provider will strip everything to the right of the pipe,
24+
# causing the command to return a full list of packages for the given action.
25+
# The trade off is to use an interpolated string knowing that action is built from an enum value and
26+
# title is pre-validated.
27+
$unless_cmd = ["/usr/bin/apt-mark ${action} ${title} | grep ${title} -q"]
28+
}
29+
30+
$onlyif_cmd = [['/usr/bin/dpkg', '-l', $title]]
31+
$command = ['/usr/bin/apt-mark', $setting, $title]
32+
33+
exec { "apt-mark ${setting} ${title}":
34+
command => $command,
35+
onlyif => $onlyif_cmd,
36+
unless => $unless_cmd,
2237
}
2338
}

0 commit comments

Comments
 (0)