Skip to content

Commit 0653649

Browse files
yuwatayuwata
committed
tree-wide: refuse too long strings earlier in specifier_printf()
We usually call specifier_printf() and then check the validity of the result. In many cases, validity checkers, e.g. path_is_valid(), refuse too long strings. This makes specifier_printf() refuse such long results earlier. Moreover, unit_full_string() and description field in sysuser now refuse results longer than LONG_LINE_MAX. config_parse() already refuses the line longer than LONG_LINE_MAX. Hence, it should be ok to set the same value as the maximum length of the resolved string.
1 parent 678d6b4 commit 0653649

File tree

15 files changed

+113
-88
lines changed

15 files changed

+113
-88
lines changed

‎src/core/load-fragment.c‎

Lines changed: 45 additions & 53 deletions
Large diffs are not rendered by default.

‎src/core/unit-printf.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,10 @@ int unit_name_printf(const Unit *u, const char* format, char **ret) {
201201
assert(format);
202202
assert(ret);
203203

204-
return specifier_printf(format, table, u, ret);
204+
return specifier_printf(format, UNIT_NAME_MAX, table, u, ret);
205205
}
206206

207-
int unit_full_printf(const Unit *u, const char *format, char **ret) {
207+
int unit_full_printf_full(const Unit *u, const char *format, size_t max_length, char **ret) {
208208
/* This is similar to unit_name_printf() but also supports unescaping. Also, adds a couple of additional codes
209209
* (which are likely not suitable for unescaped inclusion in unit names):
210210
*
@@ -265,5 +265,5 @@ int unit_full_printf(const Unit *u, const char *format, char **ret) {
265265
{}
266266
};
267267

268-
return specifier_printf(format, table, u, ret);
268+
return specifier_printf(format, max_length, table, u, ret);
269269
}

‎src/core/unit-printf.h‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,26 @@
11
/* SPDX-License-Identifier: LGPL-2.1-or-later */
22
#pragma once
33

4+
#include "creds-util.h"
5+
#include "env-util.h"
6+
#include "fd-util.h"
7+
#include "fileio.h"
48
#include "unit.h"
59

610
int unit_name_printf(const Unit *u, const char* text, char **ret);
7-
int unit_full_printf(const Unit *u, const char *text, char **ret);
11+
int unit_full_printf_full(const Unit *u, const char *text, size_t max_length, char **ret);
12+
static inline int unit_full_printf(const Unit *u, const char *text, char **ret) {
13+
return unit_full_printf_full(u, text, LONG_LINE_MAX, ret);
14+
}
15+
static inline int unit_path_printf(const Unit *u, const char *text, char **ret) {
16+
return unit_full_printf_full(u, text, PATH_MAX-1, ret);
17+
}
18+
static inline int unit_fd_printf(const Unit *u, const char *text, char **ret) {
19+
return unit_full_printf_full(u, text, FDNAME_MAX, ret);
20+
}
21+
static inline int unit_cred_printf(const Unit *u, const char *text, char **ret) {
22+
return unit_full_printf_full(u, text, CREDENTIAL_NAME_MAX, ret);
23+
}
24+
static inline int unit_env_printf(const Unit *u, const char *text, char **ret) {
25+
return unit_full_printf_full(u, text, sc_arg_max(), ret);
26+
}

‎src/partition/repart.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ static int config_parse_label(
977977
/* Nota bene: the empty label is a totally valid one. Let's hence not follow our usual rule of
978978
* assigning the empty string to reset to default here, but really accept it as label to set. */
979979

980-
r = specifier_printf(rvalue, specifier_table, NULL, &resolved);
980+
r = specifier_printf(rvalue, GPT_LABEL_MAX, specifier_table, NULL, &resolved);
981981
if (r < 0) {
982982
log_syntax(unit, LOG_WARNING, filename, line, r,
983983
"Failed to expand specifiers in Label=, ignoring: %s", rvalue);
@@ -1142,7 +1142,7 @@ static int config_parse_copy_files(
11421142
if (!isempty(p))
11431143
return log_syntax(unit, LOG_ERR, filename, line, SYNTHETIC_ERRNO(EINVAL), "Too many arguments: %s", rvalue);
11441144

1145-
r = specifier_printf(source, specifier_table, NULL, &resolved_source);
1145+
r = specifier_printf(source, PATH_MAX-1, specifier_table, NULL, &resolved_source);
11461146
if (r < 0) {
11471147
log_syntax(unit, LOG_WARNING, filename, line, r,
11481148
"Failed to expand specifiers in CopyFiles= source, ignoring: %s", rvalue);
@@ -1153,7 +1153,7 @@ static int config_parse_copy_files(
11531153
if (r < 0)
11541154
return 0;
11551155

1156-
r = specifier_printf(target, specifier_table, NULL, &resolved_target);
1156+
r = specifier_printf(target, PATH_MAX-1, specifier_table, NULL, &resolved_target);
11571157
if (r < 0) {
11581158
log_syntax(unit, LOG_WARNING, filename, line, r,
11591159
"Failed to expand specifiers in CopyFiles= target, ignoring: %s", resolved_target);
@@ -1202,7 +1202,7 @@ static int config_parse_copy_blocks(
12021202
return 0;
12031203
}
12041204

1205-
r = specifier_printf(rvalue, specifier_table, NULL, &d);
1205+
r = specifier_printf(rvalue, PATH_MAX-1, specifier_table, NULL, &d);
12061206
if (r < 0) {
12071207
log_syntax(unit, LOG_WARNING, filename, line, r,
12081208
"Failed to expand specifiers in CopyBlocks= source path, ignoring: %s", rvalue);
@@ -1250,7 +1250,7 @@ static int config_parse_make_dirs(
12501250
if (r == 0)
12511251
return 0;
12521252

1253-
r = specifier_printf(word, specifier_table, NULL, &d);
1253+
r = specifier_printf(word, PATH_MAX-1, specifier_table, NULL, &d);
12541254
if (r < 0) {
12551255
log_syntax(unit, LOG_WARNING, filename, line, r,
12561256
"Failed to expand specifiers in MakeDirectories= parameter, ignoring: %s", word);

‎src/resolve/resolved-conf.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ int config_parse_dnssd_service_name(
255255
return 0;
256256
}
257257

258-
r = specifier_printf(rvalue, specifier_table, NULL, &name);
258+
r = specifier_printf(rvalue, DNS_LABEL_MAX, specifier_table, NULL, &name);
259259
if (r < 0) {
260260
log_syntax(unit, LOG_WARNING, filename, line, r,
261261
"Invalid service instance name template '%s', ignoring assignment: %m", rvalue);

‎src/resolve/resolved-dnssd.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ int dnssd_render_instance_name(DnssdService *s, char **ret_name) {
170170
assert(s);
171171
assert(s->name_template);
172172

173-
r = specifier_printf(s->name_template, specifier_table, s, &name);
173+
r = specifier_printf(s->name_template, DNS_LABEL_MAX, specifier_table, s, &name);
174174
if (r < 0)
175175
return log_debug_errno(r, "Failed to replace specifiers: %m");
176176

‎src/shared/install-printf.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ static int specifier_last_component(char specifier, const void *data, const void
103103
return 0;
104104
}
105105

106-
int install_full_printf(const UnitFileInstallInfo *i, const char *format, char **ret) {
106+
int install_full_printf_internal(const UnitFileInstallInfo *i, const char *format, size_t max_length, char **ret) {
107107
/* This is similar to unit_name_printf() */
108108

109109
const Specifier table[] = {
@@ -123,5 +123,5 @@ int install_full_printf(const UnitFileInstallInfo *i, const char *format, char *
123123
assert(format);
124124
assert(ret);
125125

126-
return specifier_printf(format, table, i, ret);
126+
return specifier_printf(format, max_length, table, i, ret);
127127
}

‎src/shared/install-printf.h‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,12 @@
22
#pragma once
33

44
#include "install.h"
5+
#include "unit-name.h"
56

6-
int install_full_printf(const UnitFileInstallInfo *i, const char *format, char **ret);
7+
int install_full_printf_internal(const UnitFileInstallInfo *i, const char *format, size_t max_length, char **ret);
8+
static inline int install_name_printf(const UnitFileInstallInfo *i, const char *format, char **ret) {
9+
return install_full_printf_internal(i, format, UNIT_NAME_MAX, ret);
10+
}
11+
static inline int install_path_printf(const UnitFileInstallInfo *i, const char *format, char **ret) {
12+
return install_full_printf_internal(i, format, PATH_MAX-1, ret);
13+
}

‎src/shared/install.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,7 +1143,7 @@ static int config_parse_also(
11431143
if (r == 0)
11441144
break;
11451145

1146-
r = install_full_printf(info, word, &printed);
1146+
r = install_name_printf(info, word, &printed);
11471147
if (r < 0)
11481148
return r;
11491149

@@ -1190,7 +1190,7 @@ static int config_parse_default_instance(
11901190
return log_syntax(unit, LOG_WARNING, filename, line, 0,
11911191
"DefaultInstance= only makes sense for template units, ignoring.");
11921192

1193-
r = install_full_printf(i, rvalue, &printed);
1193+
r = install_name_printf(i, rvalue, &printed);
11941194
if (r < 0)
11951195
return r;
11961196

@@ -1816,7 +1816,7 @@ static int install_info_symlink_alias(
18161816
STRV_FOREACH(s, i->aliases) {
18171817
_cleanup_free_ char *alias_path = NULL, *dst = NULL, *dst_updated = NULL;
18181818

1819-
q = install_full_printf(i, *s, &dst);
1819+
q = install_path_printf(i, *s, &dst);
18201820
if (q < 0)
18211821
return q;
18221822

@@ -1891,7 +1891,7 @@ static int install_info_symlink_wants(
18911891
STRV_FOREACH(s, list) {
18921892
_cleanup_free_ char *path = NULL, *dst = NULL;
18931893

1894-
q = install_full_printf(i, *s, &dst);
1894+
q = install_name_printf(i, *s, &dst);
18951895
if (q < 0)
18961896
return q;
18971897

‎src/shared/specifier.c‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
* and "%" used for escaping. */
3030
#define POSSIBLE_SPECIFIERS ALPHANUMERICAL "%"
3131

32-
int specifier_printf(const char *text, const Specifier table[], const void *userdata, char **ret) {
32+
int specifier_printf(const char *text, size_t max_length, const Specifier table[], const void *userdata, char **ret) {
3333
size_t l, allocated = 0;
3434
_cleanup_free_ char *result = NULL;
3535
char *t;
@@ -45,7 +45,7 @@ int specifier_printf(const char *text, const Specifier table[], const void *user
4545
return -ENOMEM;
4646
t = result;
4747

48-
for (f = text; *f; f++, l--)
48+
for (f = text; *f != '\0'; f++, l--) {
4949
if (percent) {
5050
if (*f == '%')
5151
*(t++) = '%';
@@ -86,9 +86,16 @@ int specifier_printf(const char *text, const Specifier table[], const void *user
8686
else
8787
*(t++) = *f;
8888

89+
if ((size_t) (t - result) > max_length)
90+
return -ENAMETOOLONG;
91+
}
92+
8993
/* If string ended with a stray %, also end with % */
90-
if (percent)
94+
if (percent) {
9195
*(t++) = '%';
96+
if ((size_t) (t - result) > max_length)
97+
return -ENAMETOOLONG;
98+
}
9299
*(t++) = 0;
93100

94101
/* Try to deallocate unused bytes, but don't sweat it too much */

0 commit comments

Comments
 (0)