Re: [libvirt] [PATCH] maint: don't permit format strings without %

Any time we have a string with no % passed through gettext, a translator can inject a % to cause a stack overread. When there is nothing to format, it's easier to ask for a string that cannot be used as a formatter, by using a trivial "%s" format instead. In the past, we have used --disable-nls to catch some of the offenders, but that doesn't get run very often, and many more uses have crept in. Syntax check to the rescue!

The syntax check can catch uses such as virReportError(code, _("split " "string")); by using a sed script to fold context lines into one pattern space before checking for a string without %. This patch is just mechanical insertion of %s; there are probably several messages touched by this patch where we would be better off giving the user more information than a fixed string. * cfg.mk (sc_prohibit_diagnostic_without_format): New rule. * src/datatypes.c (virUnrefConnect, virGetDomain) (virUnrefDomain, virGetNetwork, virUnrefNetwork, virGetInterface) (virUnrefInterface, virGetStoragePool, virUnrefStoragePool) (virGetStorageVol, virUnrefStorageVol, virGetNodeDevice) (virGetSecret, virUnrefSecret, virGetNWFilter, virUnrefNWFilter) (virGetDomainSnapshot, virUnrefDomainSnapshot): Add %s wrapper. * src/lxc/lxc_driver.c (lxcDomainSetBlkioParameters) (lxcDomainGetBlkioParameters): Likewise. * src/conf/domain_conf.c (virSecurityDeviceLabelDefParseXML) (virDomainDiskDefParseXML, virDomainGraphicsDefParseXML): Likewise. * src/conf/network_conf.c (virNetworkDNSHostsDefParseXML) (virNetworkDefParseXML): Likewise. * src/conf/nwfilter_conf.c (virNWFilterIsValidChainName): Likewise. * src/conf/nwfilter_params.c (virNWFilterVarValueCreateSimple) (virNWFilterVarAccessParse): Likewise. * src/libvirt.c (virDomainSave, virDomainSaveFlags) (virDomainRestore, virDomainRestoreFlags) (virDomainSaveImageGetXMLDesc, virDomainSaveImageDefineXML) (virDomainCoreDump, virDomainGetXMLDesc) (virDomainMigrateVersion1, virDomainMigrateVersion2) (virDomainMigrateVersion3, virDomainMigrate, virDomainMigrate2) (virStreamSendAll, virStreamRecvAll) (virDomainSnapshotGetXMLDesc): Likewise. * src/nwfilter/nwfilter_dhcpsnoop.c (virNWFilterSnoopReqLeaseDel) (virNWFilterDHCPSnoopReq): Likewise. * src/openvz/openvz_driver.c (openvzUpdateDevice): Likewise. * src/openvz/openvz_util.c (openvzKBPerPages): Likewise. * src/qemu/qemu_cgroup.c (qemuSetupCgroup): Likewise. * src/qemu/qemu_command.c (qemuBuildHubDevStr, qemuBuildChrChardevStr) (qemuBuildCommandLine): Likewise. * src/qemu/qemu_driver.c (qemuDomainGetPercpuStats): Likewise. * src/qemu/qemu_hotplug.c (qemuDomainAttachNetDevice): Likewise. * src/rpc/virnetsaslcontext.c (virNetSASLSessionGetIdentity): Likewise. * src/rpc/virnetsocket.c (virNetSocketNewConnectUNIX) (virNetSocketSendFD, virNetSocketRecvFD): Likewise. * src/storage/storage_backend_disk.c (virStorageBackendDiskBuildPool): Likewise. * src/storage/storage_backend_fs.c (virStorageBackendFileSystemProbe) (virStorageBackendFileSystemBuild): Likewise. * src/storage/storage_backend_rbd.c (virStorageBackendRBDOpenRADOSConn): Likewise. * src/storage/storage_driver.c (storageVolumeResize): Likewise. * src/test/test_driver.c (testInterfaceChangeBegin) (testInterfaceChangeCommit, testInterfaceChangeRollback): Likewise. * src/vbox/vbox_tmpl.c (vboxListAllDomains): Likewise. * src/xenxs/xen_sxpr.c (xenFormatSxprDisk, xenFormatSxpr): Likewise. * src/xenxs/xen_xm.c (xenXMConfigGetUUID, xenFormatXMDisk) (xenFormatXM): Likewise. --- danpb asked me on IRC if it was possible to automate the checking for our recent %s additions. Of course, my answer was yes, before I knew it would take me three hours to get here... cfg.mk | 15 ++++++++++++- src/conf/domain_conf.c | 22 +++++++++---------- src/conf/network_conf.c | 8 +++---- src/conf/nwfilter_conf.c | 4 ++-- src/conf/nwfilter_params.c | 8 +++---- src/datatypes.c | 41 +++++++++++++++++++----------------- src/libvirt.c | 34 +++++++++++++++--------------- src/lxc/lxc_driver.c | 6 ++++-- src/nwfilter/nwfilter_dhcpsnoop.c | 5 +++-- src/openvz/openvz_driver.c | 2 +- src/openvz/openvz_util.c | 2 +- src/qemu/qemu_cgroup.c | 4 ++-- src/qemu/qemu_command.c | 8 +++---- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 2 +- src/rpc/virnetsaslcontext.c | 4 ++-- src/rpc/virnetsocket.c | 6 +++--- src/storage/storage_backend_disk.c | 8 +++---- src/storage/storage_backend_fs.c | 8 +++---- src/storage/storage_backend_rbd.c | 8 +++---- src/storage/storage_driver.c | 10 ++++----- src/test/test_driver.c | 6 +++--- src/vbox/vbox_tmpl.c | 2 +- src/xenxs/xen_sxpr.c | 6 +++--- src/xenxs/xen_xm.c | 8 +++---- 25 files changed, 124 insertions(+), 105 deletions(-)