SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

vmspawn: enforce an identifier grammar for QEMU config types/ids/keys

Browse Source 
qemu_config_section()/qemu_config_key() write section types and keys
unquoted ([type], key = …), but the type check only rejected '\n' while
the structurally identical id/key checks rejected more, even though the
unquoted type slot is the most sensitive. Replace the three ad-hoc
denylists with one allowlist matching QEMU's identifier grammar
([A-Za-z0-9._-], non-empty). This is merely hardening, all callers pass
constant input today.

For the value check use string_is_safe() for now, which is a bit
stricter than QEMUs own config validation.

Add test-vmspawn-qemu-config covering both.

Co-developed-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Paul Meyer <katexochen0@gmail.com>
This commit is contained in:
Paul Meyer
2026-06-17 10:23:47 +02:00
parent c55c651cc0
commit 6212baaf82
3 changed files with 140 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/vmspawn/meson.build
+5 -1
View File
@@ -7,7 +7,6 @@ endif
vmspawn_sources = files(
'vmspawn.c',
'vmspawn-bind-volume.c',
'vmspawn-qemu-config.c',
'vmspawn-qmp.c',
'vmspawn-varlink.c',
'vmspawn-settings.c',
@@ -16,6 +15,7 @@ vmspawn_sources = files(
)
vmspawn_extract_sources = files(
'vmspawn-util.c',
'vmspawn-qemu-config.c',
)
executables += [
@@ -29,4 +29,8 @@ executables += [
'sources' : files('test-vmspawn-util.c'),
'objects' : ['systemd-vmspawn'],
},
test_template + {
'sources' : files('test-vmspawn-qemu-config.c'),
'objects' : ['systemd-vmspawn'],
},
]
src/vmspawn/test-vmspawn-qemu-config.c
+120
View File
@@ -0,0 +1,120 @@
/* SPDX-License-Identifier: LGPL-2.1-or-later */
#include "alloc-util.h"
#include "memstream-util.h"
#include "tests.h"
#include "vmspawn-qemu-config.h"
/* Render a single "key = value" pair through qemu_config_key() and return both the function's return code
* and (on success, when ret is set) the rendered bytes. */
static int render_key(char **ret, const char *key, const char *value) {
_cleanup_(memstream_done) MemStream m = {};
FILE *f;
int r;
assert_se(f = memstream_init(&m));
r = qemu_config_key(f, key, value);
if (r < 0)
return r;
if (ret)
assert_se(memstream_finalize(&m, ret, NULL) >= 0);
return r;
}
static int render_section(char **ret, const char *type, const char *id) {
_cleanup_(memstream_done) MemStream m = {};
FILE *f;
int r;
assert_se(f = memstream_init(&m));
r = qemu_config_section(f, type, id);
if (r < 0)
return r;
if (ret)
assert_se(memstream_finalize(&m, ret, NULL) >= 0);
return r;
}
TEST(qemu_config_key_valid) {
_cleanup_free_ char *out = NULL;
ASSERT_OK(render_key(&out, "readonly", "on"));
ASSERT_STREQ(out, " readonly = \"on\"\n");
/* Keys may carry the full identifier charset. */
out = mfree(out);
ASSERT_OK(render_key(&out, "confidential-guest-support", "snp0"));
ASSERT_STREQ(out, " confidential-guest-support = \"snp0\"\n");
}
TEST(qemu_config_value_permits_path_bytes) {
_cleanup_free_ char *out = NULL;
/* Legitimate path bytes, including backslash, must pass through a quoted value verbatim. */
ASSERT_OK(render_key(&out, "file", "/usr/share/edk2/ovmf/OVMF_CODE.fd"));
ASSERT_STREQ(out, " file = \"/usr/share/edk2/ovmf/OVMF_CODE.fd\"\n");
out = mfree(out);
ASSERT_OK(render_key(&out, "file", "/weird/but\\legal/path"));
ASSERT_STREQ(out, " file = \"/weird/but\\legal/path\"\n");
/* '=', spaces, brackets etc. are all fine inside a quoted value. */
out = mfree(out);
ASSERT_OK(render_key(&out, "cpus", "1,sockets=1"));
ASSERT_STREQ(out, " cpus = \"1,sockets=1\"\n");
}
TEST(qemu_config_value_rejects_quote_and_newline) {
/* These two bytes are the only ones that can break out of the quoted token; both must be refused. */
ASSERT_ERROR(render_key(NULL, "file", "ab\"cd"), EINVAL);
ASSERT_ERROR(render_key(NULL, "file", "ab\ncd"), EINVAL);
}
TEST(qemu_config_key_name_rejects_structure) {
/* Key names are emitted unquoted, so they must be plain identifiers. */
ASSERT_ERROR(render_key(NULL, "fo=o", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "fo\no", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "fo\"o", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "fo o", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "foo]", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "/foo", "v"), EINVAL);
ASSERT_ERROR(render_key(NULL, "", "v"), EINVAL);
}
TEST(qemu_config_section_valid) {
_cleanup_free_ char *out = NULL;
ASSERT_OK(render_section(&out, "drive", "ovmf-code"));
ASSERT_STREQ(out, "\n[drive \"ovmf-code\"]\n");
out = mfree(out);
ASSERT_OK(render_section(&out, "smp-opts", NULL));
ASSERT_STREQ(out, "\n[smp-opts]\n");
}
TEST(qemu_config_section_type_rejects_structure) {
/* The section type is emitted unquoted as "[type]" — a ']' or newline here would let a caller
* close the header early or open a new section. */
ASSERT_ERROR(render_section(NULL, "drive]", "id"), EINVAL);
ASSERT_ERROR(render_section(NULL, "dr\nive", "id"), EINVAL);
ASSERT_ERROR(render_section(NULL, "dr ive", "id"), EINVAL);
ASSERT_ERROR(render_section(NULL, "dr\"ive", "id"), EINVAL);
ASSERT_ERROR(render_section(NULL, "", "id"), EINVAL);
}
TEST(qemu_config_section_id_rejects_structure) {
/* The id is quoted, but ']' and backslash are still hardened against future runtime data. */
ASSERT_ERROR(render_section(NULL, "drive", "i\"d"), EINVAL);
ASSERT_ERROR(render_section(NULL, "drive", "i\nd"), EINVAL);
ASSERT_ERROR(render_section(NULL, "drive", "i]d"), EINVAL);
ASSERT_ERROR(render_section(NULL, "drive", "i\\d"), EINVAL);
ASSERT_ERROR(render_section(NULL, "drive", ""), EINVAL);
}
DEFINE_TEST_MAIN(LOG_DEBUG);
src/vmspawn/vmspawn-qemu-config.c
+15 -18
View File
@@ -5,22 +5,19 @@
#include "alloc-util.h"
#include "errno-util.h"
#include "log.h"
#include "string-util.h"
#include "vmspawn-qemu-config.h"
static bool qemu_config_type_valid(const char *type) {
return !strchr(type, '\n');
}
static bool qemu_config_id_valid(const char *id) {
return !strpbrk(id, "\"\n");
}
static bool qemu_config_key_name_valid(const char *key) {
return !strpbrk(key, "=\n");
/* Enforce QEMU's identifier grammar, so runtime data can never inject config structure. */
static bool qemu_config_identifier_valid(const char *s) {
return !isempty(s) && in_charset(s, ALPHANUMERICAL ".-_");
}
/* Values are written quoted ('key = "%s"') and QEMU reads them literally between the quotes (no escapes),
* only '"' and newline can break out. Nevertheless run the value through string_is_safe(), which is
* stricter than QEMU's own parser. Relax if a valid use case occurs. */
static bool qemu_config_value_valid(const char *value) {
return !strpbrk(value, "\"\n");
return string_is_safe(value, STRING_ALLOW_EMPTY | STRING_ALLOW_BACKSLASHES | STRING_ALLOW_GLOBS);
}
int qemu_config_key(FILE *f, const char *key, const char *value) {
@@ -28,10 +25,10 @@ int qemu_config_key(FILE *f, const char *key, const char *value) {
assert(key);
assert(value);
if (!qemu_config_key_name_valid(key))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config key '%s' contains '=' or newline.", key);
if (!qemu_config_identifier_valid(key))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config key '%s' is not a valid identifier.", key);
if (!qemu_config_value_valid(value))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config value '%s' contains quote or newline.", value);
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config value '%s' contains unsafe characters.", value);
if (fprintf(f, " %s = \"%s\"\n", key, value) < 0)
return -errno_or_else(EIO);
@@ -64,12 +61,12 @@ int qemu_config_section_impl(FILE *f, const char *type, const char *id, ...) {
assert(f);
assert(type);
if (!qemu_config_type_valid(type))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config section type '%s' contains newline.", type);
if (!qemu_config_identifier_valid(type))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config section type '%s' is not a valid identifier.", type);
if (id) {
if (!qemu_config_id_valid(id))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config section id '%s' contains quote or newline.", id);
if (!qemu_config_identifier_valid(id))
return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "QEMU config section id '%s' is not a valid identifier.", id);
fprintf(f, "\n[%s \"%s\"]\n", type, id);
} else
fprintf(f, "\n[%s]\n", type);