From a43d4f543c31a17ab3a29ad8e01828ff30390622 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 19 Aug 2013 12:55:53 +0100 Subject: [PATCH] Add bounds checking on virDomain{SnapshotListAllChildren,ListAllSnapshots} RPC calls 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 --- 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 ad78011932..e228cbab79 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 33b2b0fa7e..14c16f61f2 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 eff7e1c834..9e3918c773 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; /* insert@1 */ + remote_nonnull_string names; /* 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; 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; /* insert@1 */ + remote_nonnull_string names; /* 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; int ret; };