[libvirt] [PATCH] maint: don't permit format strings without %
Daniel P. Berrange
berrange at redhat.com
Tue Jul 24 08:00:38 UTC 2012
On Mon, Jul 23, 2012 at 02:37:42PM -0600, Eric Blake wrote:
> 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!
Also with current GCC, even using '--disable-nls' doesn't produce
any errors in this regard, hence why we have so many problems.
>
> 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(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list