Skip to content

Commit 547483f

Browse files
committed
Harden db defined type
Prior to this commit there was a possibility that malformed strings could be passed in to the resource. This could lead to unsafe executions on a remote system. The parameters that are susceptible are `dbname`, `sql` and `import_cat_cmd`. 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 parameters (where possible) to ensure that the given values conform to expectation. `dbname` is validated with a regular expression that ensures that it matches expectations set by mysql. `sql` now only accepts values as an `Array[String]`. This can be one or more value. Values are also validated with a regular expression that ensures that the given paths are valid. `import_cat_cmd` has been removed in favour of a boolean parameter called `use_zcat`. Setting this parameter to `true` will set the value of the private `import_cat_cmd` to `zcat`. This commit also contains some minor refactoring.
1 parent 1469fbf commit 547483f

File tree

1 file changed

+43
-21
lines changed

1 file changed

+43
-21
lines changed

manifests/db.pp

+43-21
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@
99
# grant => ['SELECT', 'UPDATE'],
1010
# }
1111
#
12+
# @param name
13+
# The name of the database to create. Database names must:
14+
# * be longer than 64 characters.
15+
# * not contain / \ or . characters.
16+
# * not contain characters that are not permitted in file names.
17+
# * not end with space characters.
1218
# @param user
1319
# The user for the database you're creating.
1420
# @param password
@@ -28,7 +34,7 @@
2834
# @param grant_options
2935
# The grant_options for the grant for user@host on the database.
3036
# @param sql
31-
# The path to the sqlfile you want to execute. This can be single file specified as string, or it can be an array of strings.
37+
# The path to the sqlfile you want to execute. This can be a an array containing one or more file paths.
3238
# @param enforce_sql
3339
# Specifies whether executing the sqlfiles should happen on every run. If set to false, sqlfiles only run once.
3440
# @param ensure
@@ -41,25 +47,41 @@
4147
define mysql::db (
4248
$user,
4349
Variant[String, Sensitive[String]] $password,
44-
$tls_options = undef,
45-
$dbname = $name,
46-
$charset = 'utf8',
47-
$collate = 'utf8_general_ci',
48-
$host = 'localhost',
49-
$grant = 'ALL',
50-
$grant_options = undef,
51-
Optional[Variant[Array, Hash, String]] $sql = undef,
52-
$enforce_sql = false,
53-
Enum['absent', 'present'] $ensure = 'present',
54-
$import_timeout = 300,
55-
$import_cat_cmd = 'cat',
56-
$mysql_exec_path = undef,
50+
$tls_options = undef,
51+
String $dbname = $name,
52+
$charset = 'utf8',
53+
$collate = 'utf8_general_ci',
54+
$host = 'localhost',
55+
$grant = 'ALL',
56+
$grant_options = undef,
57+
Optional[Array] $sql = undef,
58+
$enforce_sql = false,
59+
Enum['absent', 'present'] $ensure = 'present',
60+
$import_timeout = 300,
61+
Enum['cat', 'zcat'] $import_cat_cmd = 'cat',
62+
$mysql_exec_path = undef,
5763
) {
58-
$table = "${dbname}.*"
64+
include 'mysql::client'
5965

60-
$sql_inputs = join([$sql], ' ')
66+
# Ensure that the database name is valid.
67+
if $dbname !~ /^[^\/?%*:|\""<>.\s;]{1,64}$/ {
68+
$message = "The database name '${dbname}' is invalid. Values must:
69+
* be longer than 64 characters.
70+
* not contain // \\ or . characters.
71+
* not contain characters that are not permitted in file names.
72+
* not end with space characters."
73+
fail($message)
74+
}
6175

62-
include 'mysql::client'
76+
# Ensure that the sql files passed are valid file paths.
77+
if $sql {
78+
$sql.each | $sqlfile | {
79+
if $sqlfile !~ /^\/(?:[A-Za-z0-9_-]+\/?+)+(?:.[A-Za-z0-9]+)$/ {
80+
$message = "The file '${sqlfile}' is invalid. A a valid file path is expected."
81+
fail($message)
82+
}
83+
}
84+
}
6385

6486
if ($mysql_exec_path) {
6587
$_mysql_exec_path = $mysql_exec_path
@@ -84,6 +106,8 @@
84106
ensure_resource('mysql_user', "${user}@${host}", $user_resource)
85107

86108
if $ensure == 'present' {
109+
$table = "${dbname}.*"
110+
87111
mysql_grant { "${user}@${host}/${table}":
88112
privileges => $grant,
89113
provider => 'mysql',
@@ -96,14 +120,12 @@
96120
],
97121
}
98122

99-
$refresh = ! $enforce_sql
100-
101123
if $sql {
102124
exec { "${dbname}-import":
103-
command => "${import_cat_cmd} ${sql_inputs} | mysql ${dbname}",
125+
command => "${import_cat_cmd} ${shell_join($sql)} | mysql ${dbname}",
104126
logoutput => true,
105127
environment => "HOME=${::root_home}",
106-
refreshonly => $refresh,
128+
refreshonly => ! $enforce_sql,
107129
path => "/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:${_mysql_exec_path}",
108130
require => Mysql_grant["${user}@${host}/${table}"],
109131
subscribe => Mysql_database[$dbname],

0 commit comments

Comments
 (0)