[PATCH] Fix JSON API

[Philipp Gesang]

Hi Gary,

a couple remarks:

-<| Quoting Gary Lockyer via samba-technical <gary at catalyst.net.nz>, on Tuesday, 2018-07-17 04:22:21 PM |>-
> diff --git a/auth/auth_log.c b/auth/auth_log.c
> index 67d23c1..bb245d8 100644
> --- a/auth/auth_log.c
> +++ b/auth/auth_log.c
> @@ -284,6 +413,17 @@ static void log_successful_authz_event_json(
>  		 DBGC_AUTH_AUDIT,
>  		 debug_level);
>  	json_free(&wrapper);
> +	return;
> +failure:
> +	/*
> +	 * On a failure authorization will not have been added to wrapper so it
> +	 * needs to be freed to avoid a leak.
> +	 *
> +	 */
> +	json_free(&authorization);
> +	json_free(&wrapper);
> +	DBG_ERR("Unable to log Authentication event JSON audit message\n");
> +	json_free(&wrapper);

Duplicate json_free().

> diff --git a/source4/dsdb/samdb/ldb_modules/audit_log.c b/source4/dsdb/samdb/ldb_modules/audit_log.c
> index 581f2f2..2b967c5 100644
> --- a/source4/dsdb/samdb/ldb_modules/audit_log.c
> +++ b/source4/dsdb/samdb/ldb_modules/audit_log.c
> @@ -341,24 +467,76 @@ static struct json_object password_change_json(
>  	unique_session_token = dsdb_audit_get_unique_session_token(module);
>  
>  	audit = json_new_object();
> -	json_add_version(&audit, PASSWORD_MAJOR, PASSWORD_MINOR);
> -	json_add_int(&audit, "statusCode", reply->error);
> +	if (json_is_invalid(&audit)) {
> +		goto failure;
> +	}
> +	rc = json_add_version(&audit, PASSWORD_MAJOR, PASSWORD_MINOR);
> +	if (rc) {
> +		goto failure;
> +	}
> +	rc = json_add_int(&audit, "statusCode", reply->error);
> +	if (rc) {
> +		goto failure;
> +	}
>  	json_add_string(&audit, "status", ldb_strerror(reply->error));

Return code of json_add_string() not handled.

> diff --git a/source4/dsdb/samdb/ldb_modules/audit_util.c b/source4/dsdb/samdb/ldb_modules/audit_util.c
> index 766c34c..a5c85d6 100644
> --- a/source4/dsdb/samdb/ldb_modules/audit_util.c
> +++ b/source4/dsdb/samdb/ldb_modules/audit_util.c
> @@ -500,15 +509,38 @@ static void dsdb_audit_add_ldb_value(
>  			len);
>  
>  		json_add_bool(&value, "base64", true);

rc = json_add_bool(&value, "base64", true);

> -		json_add_string(&value, "value", encoded);
> +		if (rc) {
> +			TALLOC_FREE(ctx);
> +			goto failure;
> +		}
> +		rc = json_add_string(&value, "value", encoded);
> +		if (rc) {
> +			TALLOC_FREE(ctx);
> +			goto failure;
> +		}
>  		TALLOC_FREE(ctx);
>  	} else {
> -		json_add_stringn(&value, "value", (char *)lv.data, len);
> +		rc = json_add_stringn(&value, "value", (char *)lv.data, len);
> +		if (rc) {
> +			goto failure;
> +		}
>  	}
>  	/*
>  	 * As array is a JSON array the element name is NULL
>  	 */
>  	json_add_object(array, NULL, &value);

rc = json_add_object(array, NULL, &value);

> +	if (rc) {
> +		goto failure;
> +	}
> +	return 0;
> +failure:
> +	/*
> +	 * In the event of a failure value will not have been added to array
> +	 * so it needs to be freed to prevent a leak.
> +	 */
> +	json_free(&value);
> +	DBG_ERR("unable to add ldb value to JSON audit message");
> +	return -1;
>  }
>  
>  /*

Btw. how about tagging the json_* functions with
__attribute__((warn_unused_result)) ?

Philipp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180717/081caf83/signature.sig>