Re: [PATCH] network: introduce APIs for writing well-formed dnsmasq.conf

10 Aug 2026

On 8/6/26 18:14, Daniel P. Berrangé via Devel wrote:
...
From: Daniel P. Berrangé <berrange@redhat.com>
In a recent commit we introduced checking for newlines in certain
dnsmasq.conf values that are expected to be fed through from
untrusted user input. Newlines could be used to inject malicious
config statements so must be forbidden.
We have many other dnsmasq.conf entries, however, that have no
checking, and while they're not considered to be security sensitive,
from a robustness POV we should exclude newlines there too to
guarantee a well-formed config file.
Instead of using virBufferAsprintf to directly format config
entries, introduce some helpers that wrap virBuffer APIs.
static void
  networkDnsmasqConfAddKey(virBuffer *buf,
                           const char *key)
  static int
  networkDnsmasqConfAddValue(virBuffer *buf,
                             const char *key,
                             const char *val)
      G_GNUC_WARN_UNUSED_RESULT;
  #define networkDnsmasqConfAddValueLit(buf, key, val) \
      ignore_value(networkDnsmasqConfAddValue(buf, key, "" val ""))
  static int
  networkDnsmasqConfAddValueFmt(virBuffer *buf,
                                const char *key,
                                const char *fmt,
                                ...)
      G_GNUC_PRINTF(3, 4)
      G_GNUC_WARN_UNUSED_RESULT;
This ensures that every config file entry we write out will
always be well-formed.
Reported-by: Vivek Parikh <vivek.parikh@breachx.ai>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/network/bridge_driver.c | 256 ++++++++++++++++++++++--------------
 1 file changed, 155 insertions(+), 101 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 6ebdc27e76..011b28b82e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -120,22 +120,6 @@ networkDnsmasqDefNamespaceFree(void *nsdata)
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(networkDnsmasqXmlNsDef, networkDnsmasqDefNamespaceFree);
-static int
-networkDnsmasqConfCheckLineBreaks(const char *record,
-                                  const char *field,
-                                  const char *value)
-{
-    if (virStringHasChars(value, "\r\n")) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("DNS %1$s record %2$s must not contain line breaks"),
-                       record, field);
-        return -1;
-    }
-
-    return 0;
-}
-
-
 static int
 networkDnsmasqDefNamespaceParseOptions(networkDnsmasqXmlNsDef *nsdef,
                                        xmlXPathContextPtr ctxt)
@@ -924,6 +908,60 @@ networkConnectSupportsFeature(virConnectPtr conn, int feature)
 }
+static void
+networkDnsmasqConfAddKey(virBuffer *buf,
+                         const char *key)
+{
+    virBufferAsprintf(buf, "%s\n", key);
+}
+
+static int
+networkDnsmasqConfAddValue(virBuffer *buf,
+                           const char *key,
+                           const char *val)
+    G_GNUC_WARN_UNUSED_RESULT;
There's no need for forward declaration. This can be rewritten as:

static int
G_GNUC_WARN_UNUSED_RESULT
networkDnsmasqConfAddValue(virBuffer *buf,
                           const char *key,
                           const char *val)
{
  ...
...
+static int
+networkDnsmasqConfAddValue(virBuffer *buf,
+                           const char *key,
+                           const char *val)
+{
+    if (virStringHasChars(val, "\r\n")) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Field '%1$s' value '%2$s' must not contain line breaks"),
+                       key, val);
+        return -1;
+    }
+
+    virBufferAsprintf(buf, "%s=%s\n", key, val);
+    return 0;
+}
+
+#define networkDnsmasqConfAddValueLit(buf, key, val) \
+    ignore_value(networkDnsmasqConfAddValue(buf, key, "" val ""))
+
+static int
+networkDnsmasqConfAddValueFmt(virBuffer *buf,
+                              const char *key,
+                              const char *fmt,
+                              ...)
+    G_GNUC_PRINTF(3, 4)
+    G_GNUC_WARN_UNUSED_RESULT;
Same here.
...
+static int
+networkDnsmasqConfAddValueFmt(virBuffer *buf,
+                              const char *key,
+                              const char *fmt,
+                              ...)
+{
+    va_list args;
+    g_autofree char *val = NULL;
+
+    va_start(args, fmt);
+    val = g_strdup_vprintf(fmt, args);
+    va_end(args);
+
+    return networkDnsmasqConfAddValue(buf, key, val);
+}
+
Please consider squashing this in:

diff --git i/src/network/bridge_driver.c w/src/network/bridge_driver.c
index 011b28b82e..c435e21bed 100644
--- i/src/network/bridge_driver.c
+++ w/src/network/bridge_driver.c
@@ -915,12 +915,9 @@ networkDnsmasqConfAddKey(virBuffer *buf,
     virBufferAsprintf(buf, "%s\n", key);
 }
 
+
 static int
-networkDnsmasqConfAddValue(virBuffer *buf,
-                           const char *key,
-                           const char *val)
-    G_GNUC_WARN_UNUSED_RESULT;
-static int
+G_GNUC_WARN_UNUSED_RESULT
 networkDnsmasqConfAddValue(virBuffer *buf,
                            const char *key,
                            const char *val)
@@ -940,13 +937,8 @@ networkDnsmasqConfAddValue(virBuffer *buf,
     ignore_value(networkDnsmasqConfAddValue(buf, key, "" val ""))
 
 static int
-networkDnsmasqConfAddValueFmt(virBuffer *buf,
-                              const char *key,
-                              const char *fmt,
-                              ...)
-    G_GNUC_PRINTF(3, 4)
-    G_GNUC_WARN_UNUSED_RESULT;
-static int
+G_GNUC_PRINTF(3, 4)
+G_GNUC_WARN_UNUSED_RESULT
 networkDnsmasqConfAddValueFmt(virBuffer *buf,
                               const char *key,
                               const char *fmt,


Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

    

Re: [PATCH] network: introduce APIs for writing well-formed dnsmasq.conf

Michal Prívozník