[libvirt] [PATCH 03/12] Add bounds checking on virDomain{SnapshotListAllChildren, ListAllSnapshots} RPC calls

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The return values for the virDomain{SnapshotListAllChildren,ListAllSnapshots} calls were not bounds checked. This is a robustness issue for clients if something where to cause corruption of the RPC stream data. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- daemon/remote.c | 14 ++++++++++++++ src/remote/remote_driver.c | 16 ++++++++++++++++ src/remote/remote_protocol.x | 10 +++++----- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index ad78011..e228cba 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -3934,6 +3934,13 @@ remoteDispatchDomainListAllSnapshots(virNetServerPtr server ATTRIBUTE_UNUSED, args->flags)) < 0) goto cleanup; + if (nsnaps > REMOTE_DOMAIN_SNAPSHOT_LIST_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many domain snapshots '%d' for limit '%d'"), + nsnaps, REMOTE_DOMAIN_SNAPSHOT_LIST_MAX); + goto cleanup; + } + if (snaps && nsnaps) { if (VIR_ALLOC_N(ret->snapshots.snapshots_val, nsnaps) < 0) goto cleanup; @@ -3996,6 +4003,13 @@ remoteDispatchDomainSnapshotListAllChildren(virNetServerPtr server ATTRIBUTE_UNU args->flags)) < 0) goto cleanup; + if (nsnaps > REMOTE_DOMAIN_SNAPSHOT_LIST_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many domain snapshots '%d' for limit '%d'"), + nsnaps, REMOTE_DOMAIN_SNAPSHOT_LIST_MAX); + goto cleanup; + } + if (snaps && nsnaps) { if (VIR_ALLOC_N(ret->snapshots.snapshots_val, nsnaps) < 0) goto cleanup; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 33b2b0f..14c16f6 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -5760,6 +5760,14 @@ remoteDomainListAllSnapshots(virDomainPtr dom, (char *) &ret) == -1) goto done; + if (ret.snapshots.snapshots_len > REMOTE_DOMAIN_SNAPSHOT_LIST_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many domain snapshots '%d' for limit '%d'"), + ret.snapshots.snapshots_len, + REMOTE_DOMAIN_SNAPSHOT_LIST_MAX); + goto cleanup; + } + if (snapshots) { if (VIR_ALLOC_N(snaps, ret.snapshots.snapshots_len + 1) < 0) goto cleanup; @@ -5819,6 +5827,14 @@ remoteDomainSnapshotListAllChildren(virDomainSnapshotPtr parent, (char *) &ret) == -1) goto done; + if (ret.snapshots.snapshots_len > REMOTE_DOMAIN_SNAPSHOT_LIST_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many domain snapshots '%d' for limit '%d'"), + ret.snapshots.snapshots_len, + REMOTE_DOMAIN_SNAPSHOT_LIST_MAX); + goto cleanup; + } + if (snapshots) { if (VIR_ALLOC_N(snaps, ret.snapshots.snapshots_len + 1) < 0) goto cleanup; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index eff7e1c..9e3918c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -154,7 +154,7 @@ const REMOTE_AUTH_TYPE_LIST_MAX = 20; const REMOTE_DOMAIN_MEMORY_STATS_MAX = 1024; /* Upper limit on lists of domain snapshots. */ -const REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX = 1024; +const REMOTE_DOMAIN_SNAPSHOT_LIST_MAX = 1024; /* Maximum length of a block peek buffer message. * Note applications need to be aware of this limit and issue multiple @@ -2396,7 +2396,7 @@ struct remote_domain_snapshot_list_names_args { }; struct remote_domain_snapshot_list_names_ret { - remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX>; /* insert@1 */ + remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_MAX>; /* insert@1 */ }; struct remote_domain_list_all_snapshots_args { @@ -2406,7 +2406,7 @@ struct remote_domain_list_all_snapshots_args { }; struct remote_domain_list_all_snapshots_ret { - remote_nonnull_domain_snapshot snapshots<>; + remote_nonnull_domain_snapshot snapshots<REMOTE_DOMAIN_SNAPSHOT_LIST_MAX>; int ret; }; @@ -2426,7 +2426,7 @@ struct remote_domain_snapshot_list_children_names_args { }; struct remote_domain_snapshot_list_children_names_ret { - remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX>; /* insert@1 */ + remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_MAX>; /* insert@1 */ }; struct remote_domain_snapshot_list_all_children_args { @@ -2436,7 +2436,7 @@ struct remote_domain_snapshot_list_all_children_args { }; struct remote_domain_snapshot_list_all_children_ret { - remote_nonnull_domain_snapshot snapshots<>; + remote_nonnull_domain_snapshot snapshots<REMOTE_DOMAIN_SNAPSHOT_LIST_MAX>; int ret; }; -- 1.8.3.1