Re: [libvirt] [PATCH v2] XML: escape strings where we should do it

There is a lot of places, were it's pretty ease for user to enter some characters that we need to escape to create a valid XML description. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1197580 Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- Previous version: https://www.redhat.com/archives/libvir-list/2015-May/msg00110.html I've went through all the usage of virBufferAsprintf and hopefully I didn't miss any wrong usage. src/conf/capabilities.c | 6 ++-- src/conf/cpu_conf.c | 6 ++-- src/conf/domain_capabilities.c | 2 +- src/conf/domain_conf.c | 63 +++++++++++++++++++++--------------------- src/conf/network_conf.c | 17 ++++++------ src/conf/node_device_conf.c | 4 +-- 6 files changed, 49 insertions(+), 49 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index c43bfb3..6bd06e5 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -682,9 +682,9 @@ virCapabilitiesDomainDataLookupInternal(virCapsPtr caps, virBufferAsprintf(&buf, "domaintype=%s ", virDomainVirtTypeToString(domaintype)); if (emulator) - virBufferAsprintf(&buf, "emulator=%s ", emulator); + virBufferEscapeString(&buf, "emulator=%s ", emulator);

if (machinetype) - virBufferAsprintf(&buf, "machine=%s ", machinetype); + virBufferEscapeString(&buf, "machine=%s ", machinetype); if (virBufferCurrentContent(&buf) && !virBufferCurrentContent(&buf)[0]) virBufferAsprintf(&buf, "%s", _("any configuration")); @@ -903,7 +903,7 @@ virCapabilitiesFormatXML(virCapsPtr caps) virBufferAdjustIndent(&buf, 2); for (i = 0; i < caps->host.nmigrateTrans; i++) { virBufferAsprintf(&buf, "<uri_transport>%s</uri_transport>\n", - caps->host.migrateTrans[i]); + caps->host.migrateTrans[i]);

} virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</uri_transports>\n"); diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index e959ecc..1ba1d82 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -544,17 +544,17 @@ virCPUDefFormatBuf(virBufferPtr buf, } virBufferAsprintf(buf, " fallback='%s'", fallback); if (def->vendor_id) - virBufferAsprintf(buf, " vendor_id='%s'", def->vendor_id); + virBufferEscapeString(buf, " vendor_id='%s'", def->vendor_id); } if (formatModel && def->model) { - virBufferAsprintf(buf, ">%s</model>\n", def->model); + virBufferEscapeString(buf, ">%s</model>\n", def->model);

} else { virBufferAddLit(buf, "/>\n"); } } if (formatModel && def->vendor) - virBufferAsprintf(buf, "<vendor>%s</vendor>\n", def->vendor); + virBufferEscapeString(buf, "<vendor>%s</vendor>\n", def->vendor);

if (def->sockets && def->cores && def->threads) { virBufferAddLit(buf, "<topology"); diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 7c59912..0e32f52 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -272,7 +272,7 @@ virDomainCapsFormatInternal(virBufferPtr buf, virBufferAddLit(buf, "<domainCapabilities>\n"); virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<path>%s</path>\n", caps->path); + virBufferEscapeString(buf, "<path>%s</path>\n", caps->path); virBufferAsprintf(buf, "<domain>%s</domain>\n", virttype_str); virBufferAsprintf(buf, "<machine>%s</machine>\n", caps->machine); virBufferAsprintf(buf, "<arch>%s</arch>\n", arch_str); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e3e0f63..4c3f46f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3724,7 +3724,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf, virBufferAsprintf(buf, " bar='%s'", rombar); } if (info->romfile) - virBufferAsprintf(buf, " file='%s'", info->romfile); + virBufferEscapeString(buf, " file='%s'", info->romfile); virBufferAddLit(buf, "/>\n"); } @@ -17712,7 +17712,7 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf, virBufferAddLit(buf, "<seclabel"); if (def->model) - virBufferAsprintf(buf, " model='%s'", def->model); + virBufferEscapeString(buf, " model='%s'", def->model); if (def->labelskip) virBufferAddLit(buf, " labelskip='yes'"); @@ -19246,50 +19246,51 @@ virDomainChrSourceDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_CHR_TYPE_NMDM: - virBufferAsprintf(buf, "<source master='%s' slave='%s'/>\n", - def->data.nmdm.master, - def->data.nmdm.slave); + virBufferEscapeString(buf, "<source master='%s' ", + def->data.nmdm.master); + virBufferEscapeString(buf, "slave='%s'/>\n", def->data.nmdm.slave); break; case VIR_DOMAIN_CHR_TYPE_UDP: if (def->data.udp.bindService && def->data.udp.bindHost) { - virBufferAsprintf(buf, - "<source mode='bind' host='%s' " - "service='%s'/>\n", - def->data.udp.bindHost, - def->data.udp.bindService); + virBufferEscapeString(buf, + "<source mode='bind' host='%s' ", + def->data.udp.bindHost); + virBufferEscapeString(buf, + "service='%s'/>\n", + def->data.udp.bindService); } else if (def->data.udp.bindHost) { - virBufferAsprintf(buf, "<source mode='bind' host='%s'/>\n", - def->data.udp.bindHost); + virBufferEscapeString(buf, "<source mode='bind' host='%s'/>\n", + def->data.udp.bindHost); } else if (def->data.udp.bindService) { - virBufferAsprintf(buf, "<source mode='bind' service='%s'/>\n", - def->data.udp.bindService); + virBufferEscapeString(buf, "<source mode='bind' service='%s'/>\n", + def->data.udp.bindService); } if (def->data.udp.connectService && def->data.udp.connectHost) { - virBufferAsprintf(buf, - "<source mode='connect' host='%s' " - "service='%s'/>\n", - def->data.udp.connectHost, - def->data.udp.connectService); + virBufferEscapeString(buf, + "<source mode='connect' host='%s' ", + def->data.udp.connectHost); + virBufferEscapeString(buf, + "service='%s'/>\n", + def->data.udp.connectService); } else if (def->data.udp.connectHost) { - virBufferAsprintf(buf, "<source mode='connect' host='%s'/>\n", - def->data.udp.connectHost); + virBufferEscapeString(buf, "<source mode='connect' host='%s'/>\n", + def->data.udp.connectHost); } else if (def->data.udp.connectService) { - virBufferAsprintf(buf, - "<source mode='connect' service='%s'/>\n", - def->data.udp.connectService); + virBufferEscapeString(buf, + "<source mode='connect' service='%s'/>\n", + def->data.udp.connectService); } break;

case VIR_DOMAIN_CHR_TYPE_TCP: - virBufferAsprintf(buf, - "<source mode='%s' host='%s' service='%s'/>\n", - def->data.tcp.listen ? "bind" : "connect", - def->data.tcp.host, - def->data.tcp.service); + virBufferAsprintf(buf, "<source mode='%s' ", + def->data.tcp.listen ? "bind" : "connect"); + virBufferEscapeString(buf, "host='%s' ", def->data.tcp.host); + virBufferEscapeString(buf, "service='%s'/>\n", def->data.tcp.service); virBufferAsprintf(buf, "<protocol type='%s'/>\n", virDomainChrTcpProtocolTypeToString( def->data.tcp.protocol)); @@ -19303,8 +19304,8 @@ virDomainChrSourceDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_CHR_TYPE_SPICEPORT: - virBufferAsprintf(buf, "<source channel='%s'/>\n", - def->data.spiceport.channel); + virBufferEscapeString(buf, "<source channel='%s'/>\n", + def->data.spiceport.channel); break; } diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 5b734f2..aa607a8 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2422,21 +2422,20 @@ virNetworkDNSDefFormat(virBufferPtr buf, } for (i = 0; i < def->ntxts; i++) { - virBufferAsprintf(buf, "<txt name='%s' value='%s'/>\n", - def->txts[i].name, - def->txts[i].value); + virBufferEscapeString(buf, "<txt name='%s' ", def->txts[i].name); + virBufferEscapeString(buf, "value='%s'/>\n", def->txts[i].value); } for (i = 0; i < def->nsrvs; i++) { if (def->srvs[i].service && def->srvs[i].protocol) { - virBufferAsprintf(buf, "<srv service='%s' protocol='%s'", - def->srvs[i].service, - def->srvs[i].protocol); + virBufferEscapeString(buf, "<srv service='%s' ", + def->srvs[i].service); + virBufferEscapeString(buf, "protocol='%s'", def->srvs[i].protocol); if (def->srvs[i].domain) virBufferAsprintf(buf, " domain='%s'", def->srvs[i].domain);

if (def->srvs[i].target) - virBufferAsprintf(buf, " target='%s'", def->srvs[i].target); + virBufferEscapeString(buf, " target='%s'", def->srvs[i].target);

if (def->srvs[i].port) virBufferAsprintf(buf, " port='%d'", def->srvs[i].port); if (def->srvs[i].priority) @@ -2455,8 +2454,8 @@ virNetworkDNSDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<host ip='%s'>\n", ip); virBufferAdjustIndent(buf, 2); for (j = 0; j < def->hosts[i].nnames; j++) - virBufferAsprintf(buf, "<hostname>%s</hostname>\n", - def->hosts[i].names[j]); + virBufferEscapeString(buf, "<hostname>%s</hostname>\n", + def->hosts[i].names[j]); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</host>\n"); diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index a286847..feae3d4 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -514,8 +514,8 @@ char *virNodeDeviceDefFormat(const virNodeDeviceDef *def) virBufferEscapeString(&buf, "<vendor>%s</vendor>\n", data->storage.vendor); if (data->storage.serial) - virBufferAsprintf(&buf, "<serial>%s</serial>\n", - data->storage.serial); + virBufferEscapeString(&buf, "<serial>%s</serial>\n", + data->storage.serial); if (data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_REMOVABLE) { int avl = data->storage.flags & VIR_NODE_DEV_CAP_STORAGE_REMOVABLE_MEDIA_AVAILABLE;