If virJSONValueObjectGetArray fails to find "members" in @localroot,
then using @rootmembers in subsequent calls which assume that it was
successful will not go well. So add a check that it was successfully
fetched and an error if not.
Similarly virJSONValueArraySize returns an 'ssize_t', so the callers
should use the right type and they should check the return value
against -1 and print an error if something is wrong.
Found by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
tests/testutilsqemuschema.c | 40 +++++++++++++++++++++++++++++-----------
1 file changed, 29 insertions(+), 11 deletions(-)
diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
index 21f5d119e8..79c526ac7c 100644
--- a/tests/testutilsqemuschema.c
+++ b/tests/testutilsqemuschema.c
@@ -97,14 +97,19 @@ struct testQEMUSchemaValidateObjectMemberData {
static virJSONValuePtr
testQEMUSchemaStealObjectMemberByName(const char *name,
- virJSONValuePtr members)
+ virJSONValuePtr members,
+ virBufferPtr debug)
{
virJSONValuePtr member;
virJSONValuePtr ret = NULL;
- size_t n;
+ ssize_t n;
size_t i;
- n = virJSONValueArraySize(members);
+ if ((n = virJSONValueArraySize(members)) < 0) {
+ virBufferAddLit(debug, "ERROR: members size < 0");
+ return NULL;
+ }
+
for (i = 0; i < n; i++) {
member = virJSONValueArrayGet(members, i);
@@ -132,7 +137,7 @@ testQEMUSchemaValidateObjectMember(const char *key,
virBufferStrcat(data->debug, key, ": ", NULL);
/* lookup 'member' entry for key */
- if (!(keymember = testQEMUSchemaStealObjectMemberByName(key, data->rootmembers))) {
+ if (!(keymember = testQEMUSchemaStealObjectMemberByName(key, data->rootmembers, data->debug))) {
virBufferAddLit(data->debug, "ERROR: attribute not in schema");
goto cleanup;
}
@@ -188,7 +193,7 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr root,
virHashTablePtr schema,
virBufferPtr debug)
{
- size_t n;
+ ssize_t n;
size_t i;
virJSONValuePtr variants = NULL;
virJSONValuePtr variant;
@@ -203,7 +208,11 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr root,
return -2;
}
- n = virJSONValueArraySize(variants);
+ if ((n = virJSONValueArraySize(variants)) < 0) {
+ virBufferAddLit(debug, "ERROR: 'variants' array size < 0");
+ return -2;
+ }
+
for (i = 0; i < n; i++) {
variant = virJSONValueArrayGet(variants, i);
@@ -307,7 +316,10 @@ testQEMUSchemaValidateObject(virJSONValuePtr obj,
/* validate members */
- data.rootmembers = virJSONValueObjectGetArray(localroot, "members");
+ if (!(data.rootmembers = virJSONValueObjectGetArray(localroot, "members"))) {
+ virBufferAddLit(debug, "ERROR: missing attribute 'members'\n");
+ goto cleanup;
+ }
if (virJSONValueObjectForeachKeyValue(obj,
testQEMUSchemaValidateObjectMember,
&data) < 0)
@@ -342,7 +354,7 @@ testQEMUSchemaValidateEnum(virJSONValuePtr obj,
const char *objstr;
virJSONValuePtr values = NULL;
virJSONValuePtr value;
- size_t n;
+ ssize_t n;
size_t i;
if (virJSONValueGetType(obj) != VIR_JSON_TYPE_STRING) {
@@ -358,7 +370,10 @@ testQEMUSchemaValidateEnum(virJSONValuePtr obj,
return -2;
}
- n = virJSONValueArraySize(values);
+ if ((n = virJSONValueArraySize(values)) < 0) {
+ virBufferAddLit(debug, "ERROR: enum values array size < 0");
+ return -2;
+ }
for (i = 0; i < n; i++) {
value = virJSONValueArrayGet(values, i);
@@ -423,7 +438,7 @@ testQEMUSchemaValidateAlternate(virJSONValuePtr obj,
{
virJSONValuePtr members;
virJSONValuePtr member;
- size_t n;
+ ssize_t n;
size_t i;
const char *membertype;
virJSONValuePtr memberschema;
@@ -439,7 +454,10 @@ testQEMUSchemaValidateAlternate(virJSONValuePtr obj,
virBufferAdjustIndent(debug, 3);
indent = virBufferGetIndent(debug, false);
- n = virJSONValueArraySize(members);
+ if ((n = virJSONValueArraySize(members)) < 0) {
+ virBufferAddLit(debug, "ERROR: alternate schema 'members' array size < 0");
+ return -2;
+ }
for (i = 0; i < n; i++) {
membertype = NULL;
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Apr 17, 2018 at 12:22:16 -0400, John Ferlan wrote:
> If virJSONValueObjectGetArray fails to find "members" in @localroot,
> then using @rootmembers in subsequent calls which assume that it was
> successful will not go well. So add a check that it was successfully
> fetched and an error if not.
>
> Similarly virJSONValueArraySize returns an 'ssize_t', so the callers
> should use the right type and they should check the return value
> against -1 and print an error if something is wrong.
>
> Found by Coverity
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> tests/testutilsqemuschema.c | 40 +++++++++++++++++++++++++++++-----------
> 1 file changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/tests/testutilsqemuschema.c b/tests/testutilsqemuschema.c
> index 21f5d119e8..79c526ac7c 100644
> --- a/tests/testutilsqemuschema.c
> +++ b/tests/testutilsqemuschema.c
> @@ -97,14 +97,19 @@ struct testQEMUSchemaValidateObjectMemberData {
>
> static virJSONValuePtr
> testQEMUSchemaStealObjectMemberByName(const char *name,
> - virJSONValuePtr members)
> + virJSONValuePtr members,
> + virBufferPtr debug)
> {
> virJSONValuePtr member;
> virJSONValuePtr ret = NULL;
> - size_t n;
> + ssize_t n;
> size_t i;
>
> - n = virJSONValueArraySize(members);
> + if ((n = virJSONValueArraySize(members)) < 0) {
> + virBufferAddLit(debug, "ERROR: members size < 0");
> + return NULL;
> + }
> +
> for (i = 0; i < n; i++) {
> member = virJSONValueArrayGet(members, i);
>
> @@ -132,7 +137,7 @@ testQEMUSchemaValidateObjectMember(const char *key,
> virBufferStrcat(data->debug, key, ": ", NULL);
>
> /* lookup 'member' entry for key */
> - if (!(keymember = testQEMUSchemaStealObjectMemberByName(key, data->rootmembers))) {
> + if (!(keymember = testQEMUSchemaStealObjectMemberByName(key, data->rootmembers, data->debug))) {
> virBufferAddLit(data->debug, "ERROR: attribute not in schema");
> goto cleanup;
All of this bove should be fixed by checking the return value of
virJSONValueObjectGetArray in testQEMUSchemaValidateObject where
data.rootmembers.
This will make sure that the callee gets an array and thus the array
size function will not return -1;
> }
[...]
> @@ -203,7 +208,11 @@ testQEMUSchemaValidateObjectMergeVariant(virJSONValuePtr root,
> return -2;
> }
>
> - n = virJSONValueArraySize(variants);
'variants' was retrieved via virJSONValueObjectStealArray thus this will
not return -1 as it was type-checked.
> + if ((n = virJSONValueArraySize(variants)) < 0) {
> + virBufferAddLit(debug, "ERROR: 'variants' array size < 0");
> + return -2;
> + }
> +
> for (i = 0; i < n; i++) {
> variant = virJSONValueArrayGet(variants, i);
>
> @@ -307,7 +316,10 @@ testQEMUSchemaValidateObject(virJSONValuePtr obj,
>
>
> /* validate members */
> - data.rootmembers = virJSONValueObjectGetArray(localroot, "members");
> + if (!(data.rootmembers = virJSONValueObjectGetArray(localroot, "members"))) {
> + virBufferAddLit(debug, "ERROR: missing attribute 'members'\n");
> + goto cleanup;
> + }
> if (virJSONValueObjectForeachKeyValue(obj,
> testQEMUSchemaValidateObjectMember,
> &data) < 0)
[...]
> @@ -358,7 +370,10 @@ testQEMUSchemaValidateEnum(virJSONValuePtr obj,
> return -2;
> }
>
> - n = virJSONValueArraySize(values);
'values' was retrieved via virJSONValueObjectGetArray thus this will
not return -1 as it was type-checked.
> + if ((n = virJSONValueArraySize(values)) < 0) {
> + virBufferAddLit(debug, "ERROR: enum values array size < 0");
> + return -2;
> + }
> for (i = 0; i < n; i++) {
> value = virJSONValueArrayGet(values, i);
>
[...]
> @@ -439,7 +454,10 @@ testQEMUSchemaValidateAlternate(virJSONValuePtr obj,
> virBufferAdjustIndent(debug, 3);
> indent = virBufferGetIndent(debug, false);
>
> - n = virJSONValueArraySize(members);
'members' was retrieved via virJSONValueObjectGetArray thus this will
not return -1 as it was type-checked.
> + if ((n = virJSONValueArraySize(members)) < 0) {
> + virBufferAddLit(debug, "ERROR: alternate schema 'members' array size < 0");
> + return -2;
> + }
> for (i = 0; i < n; i++) {
> membertype = NULL;
Also all of the error messages are misleading. The schema can't have an
array with < 0 elements, since that is not possible in JSON. The problem
always will be that the field is either missing or of incorrect type.
Rather than adding pointless checks it might be worth just to modify
virJSONValueArraySize so that it does not do type checking and offload
that to the caller.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.