From f73c042adc97544c10542c4818afb703cc20ed7a Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Mon, 3 Feb 2025 16:19:25 +0100 Subject: [PATCH v47 06/11] fixup! Add OAUTHBEARER SASL mechanism --- src/interfaces/libpq/fe-auth-oauth-curl.c | 74 ++++++++++++++++++----- src/interfaces/libpq/fe-auth-oauth.c | 14 ++++- 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/src/interfaces/libpq/fe-auth-oauth-curl.c b/src/interfaces/libpq/fe-auth-oauth-curl.c index b6255834f00..6c8333789f1 100644 --- a/src/interfaces/libpq/fe-auth-oauth-curl.c +++ b/src/interfaces/libpq/fe-auth-oauth-curl.c @@ -461,12 +461,15 @@ oauth_json_object_field_start(void *state, char *name, bool isnull) /* * We should never start parsing a new field while a previous one is * still active. - * - * TODO: this code relies on assertions too much. We need to exit - * sanely on internal logic errors, to avoid turning bugs into - * vulnerabilities. */ - Assert(!ctx->active); + if (ctx->active) + { + Assert(false); + oauth_parse_set_error(ctx, + "internal error: started field '%s' before field '%s' was finished", + name, ctx->active->name); + return JSON_SEM_ACTION_FAILED; + } while (field->name) { @@ -506,8 +509,19 @@ oauth_json_object_end(void *state) struct oauth_parse *ctx = state; --ctx->nested; - if (!ctx->nested) - Assert(!ctx->active); /* all fields should be fully processed */ + + /* + * All fields should be fully processed by the end of the top-level + * object. + */ + if (!ctx->nested && ctx->active) + { + Assert(false); + oauth_parse_set_error(ctx, + "internal error: field '%s' still active at end of object", + ctx->active->name); + return JSON_SEM_ACTION_FAILED; + } return JSON_SUCCESS; } @@ -546,11 +560,18 @@ oauth_json_array_end(void *state) if (ctx->active) { /* - * This assumes that no target arrays can contain other arrays, which - * we check in the array_start callback. + * Clear the target (which should be an array inside the top-level + * object). For this to be safe, no target arrays can contain other + * arrays; we check for that in the array_start callback. */ - Assert(ctx->nested == 2); - Assert(ctx->active->type == JSON_TOKEN_ARRAY_START); + if (ctx->nested != 2 || ctx->active->type != JSON_TOKEN_ARRAY_START) + { + Assert(false); + oauth_parse_set_error(ctx, + "internal error: found unexpected array end while parsing field '%s'", + ctx->active->name); + return JSON_SEM_ACTION_FAILED; + } ctx->active = NULL; } @@ -597,8 +618,25 @@ oauth_json_scalar(void *state, char *token, JsonTokenType type) if (field->type != JSON_TOKEN_ARRAY_START) { - Assert(ctx->nested == 1); - Assert(!*field->target.scalar); + /* Ensure that we're parsing the top-level keys... */ + if (ctx->nested != 1) + { + Assert(false); + oauth_parse_set_error(ctx, + "internal error: scalar target found at nesting level %d", + ctx->nested); + return JSON_SEM_ACTION_FAILED; + } + + /* ...and that a result has not already been set. */ + if (*field->target.scalar) + { + Assert(false); + oauth_parse_set_error(ctx, + "internal error: scalar field '%s' would be assigned twice", + ctx->active->name); + return JSON_SEM_ACTION_FAILED; + } *field->target.scalar = strdup(token); if (!*field->target.scalar) @@ -612,7 +650,15 @@ oauth_json_scalar(void *state, char *token, JsonTokenType type) { struct curl_slist *temp; - Assert(ctx->nested == 2); + /* The target array should be inside the top-level object. */ + if (ctx->nested != 2) + { + Assert(false); + oauth_parse_set_error(ctx, + "internal error: array member found at nesting level %d", + ctx->nested); + return JSON_SEM_ACTION_FAILED; + } /* Note that curl_slist_append() makes a copy of the token. */ temp = curl_slist_append(*field->target.array, token); diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c index cc53e2bdd1a..8beae9604c7 100644 --- a/src/interfaces/libpq/fe-auth-oauth.c +++ b/src/interfaces/libpq/fe-auth-oauth.c @@ -208,6 +208,7 @@ oauth_json_object_field_start(void *state, char *name, bool isnull) { struct json_ctx *ctx = state; + /* Only top-level keys are considered. */ if (ctx->nested == 1) { if (strcmp(name, ERROR_STATUS_FIELD) == 0) @@ -264,7 +265,18 @@ oauth_json_scalar(void *state, char *token, JsonTokenType type) if (ctx->target_field) { - Assert(ctx->nested == 1); + if (ctx->nested != 1) + { + /* + * ctx->target_field should not have been set for nested keys. + * Assert and don't continue any further for production builds. + */ + Assert(false); + oauth_json_set_error(ctx, /* don't bother translating */ + "internal error: target scalar found at nesting level %d during OAUTHBEARER parsing", + ctx->nested); + return JSON_SEM_ACTION_FAILED; + } /* * We don't allow duplicate field names; error out if the target has -- 2.34.1