From d102cbc27220e62451dd4dc5b0092a6fc16edba3 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Wed, 16 Nov 2022 15:40:19 +0100 Subject: [PATCH 4/4] Replace all use of xmlSecBase64Decode by lasso_base64_decode (#71313) --- lasso/id-ff/login.c | 12 ++- lasso/id-ff/provider.c | 8 +- lasso/id-ff/session.c | 15 ++-- lasso/saml-2.0/profile.c | 35 ++++---- lasso/xml/tools.c | 172 ++++++++++++++++----------------------- lasso/xml/xml.c | 16 ++-- 6 files changed, 111 insertions(+), 147 deletions(-) diff --git a/lasso/id-ff/login.c b/lasso/id-ff/login.c index f1f1bce3..d5559901 100644 --- a/lasso/id-ff/login.c +++ b/lasso/id-ff/login.c @@ -1652,7 +1652,8 @@ lasso_login_init_request(LassoLogin *login, gchar *response_msg, int i; char *artifact_b64 = NULL, *provider_succinct_id_b64; char provider_succinct_id[21]; - char artifact[43]; + char *artifact = NULL; + int artifact_len = 0; LassoSamlpRequestAbstract *request; LassoProfile *profile; @@ -1689,18 +1690,23 @@ lasso_login_init_request(LassoLogin *login, gchar *response_msg, lasso_assign_string(artifact_b64, response_msg); } - i = xmlSecBase64Decode((xmlChar*)artifact_b64, (xmlChar*)artifact, 43); - if (i < 0 || i > 42) { + if (! lasso_base64_decode(artifact_b64, &artifact, &artifact_len)) { + return LASSO_PROFILE_ERROR_INVALID_ARTIFACT; + } + if (artifact_len != 42) { lasso_release_string(artifact_b64); + lasso_release_string(artifact); return LASSO_PROFILE_ERROR_INVALID_ARTIFACT; } if (artifact[0] != 0 || artifact[1] != 3) { /* wrong type code */ lasso_release_string(artifact_b64); + lasso_release_string(artifact); return LASSO_PROFILE_ERROR_INVALID_ARTIFACT; } memcpy(provider_succinct_id, artifact+2, 20); + lasso_release_string(artifact); provider_succinct_id[20] = 0; provider_succinct_id_b64 = (char*)xmlSecBase64Encode((xmlChar*)provider_succinct_id, 20, 0); diff --git a/lasso/id-ff/provider.c b/lasso/id-ff/provider.c index e6bd6ba9..e789815c 100644 --- a/lasso/id-ff/provider.c +++ b/lasso/id-ff/provider.c @@ -1434,12 +1434,12 @@ lasso_provider_verify_signature(LassoProvider *provider, if (format == LASSO_MESSAGE_FORMAT_BASE64) { int len; - char *msg = g_malloc(strlen(message)); - len = xmlSecBase64Decode((xmlChar*)message, (xmlChar*)msg, strlen(message)); - if (len < 0) { + char *msg = NULL; + + if (! lasso_base64_decode(message, &msg, &len)) { goto_cleanup_with_rc(LASSO_PROFILE_ERROR_INVALID_MSG); } - doc = lasso_xml_parse_memory(msg, strlen(msg)); + doc = lasso_xml_parse_memory(msg, len); lasso_release_string(msg); } else { doc = lasso_xml_parse_memory(message, strlen(message)); diff --git a/lasso/id-ff/session.c b/lasso/id-ff/session.c index e22a5e80..c303470c 100644 --- a/lasso/id-ff/session.c +++ b/lasso/id-ff/session.c @@ -797,25 +797,22 @@ get_xmlNode(LassoNode *node, G_GNUC_UNUSED gboolean lasso_dump) xmlNode* base64_to_xmlNode(xmlChar *buffer) { - xmlChar *decoded = NULL; + char *decoded = NULL; + int decoded_len = 0; xmlDoc *doc = NULL; xmlNode *ret = NULL; - int l1,l2; - l1 = 4*strlen((char*)buffer)+2; - decoded = g_malloc(l1); - l2 = xmlSecBase64Decode(buffer, decoded, l1); - if (l2 < 0) + if (! lasso_base64_decode((char*)buffer, &decoded, &decoded_len)) goto cleanup; - doc = xmlParseMemory((char*)decoded, l2); + doc = xmlParseMemory(decoded, decoded_len); if (doc == NULL) goto cleanup; ret = xmlDocGetRootElement(doc); if (ret) { - ret = xmlCopyNode(ret, 1); + ret = xmlCopyNode(ret, 1); } cleanup: - lasso_release(decoded); + lasso_release_string(decoded); lasso_release_doc(doc); return ret; diff --git a/lasso/saml-2.0/profile.c b/lasso/saml-2.0/profile.c index 9f2b5156..378f072d 100644 --- a/lasso/saml-2.0/profile.c +++ b/lasso/saml-2.0/profile.c @@ -288,7 +288,8 @@ lasso_saml20_profile_init_artifact_resolve(LassoProfile *profile, char *artifact_b64 = NULL; xmlChar *provider_succinct_id_b64 = NULL; char *provider_succinct_id[21]; - char artifact[45]; + char *artifact = NULL; + int artifact_len = 0; LassoSamlp2RequestAbstract *request = NULL; LassoProvider *remote_provider = NULL; int i = 0; @@ -307,21 +308,17 @@ lasso_saml20_profile_init_artifact_resolve(LassoProfile *profile, return LASSO_PROFILE_ERROR_MISSING_ARTIFACT; } } else if (method == LASSO_HTTP_METHOD_ARTIFACT_POST) { - artifact_b64 = g_strdup(msg); + lasso_assign_string(artifact_b64, msg); } else { return critical_error(LASSO_PROFILE_ERROR_INVALID_HTTP_METHOD); } - i = xmlSecBase64Decode((xmlChar*)artifact_b64, (xmlChar*)artifact, 45); - if (i < 0 || i > 44) { - lasso_release_string(artifact_b64); - return LASSO_PROFILE_ERROR_INVALID_ARTIFACT; - } + goto_cleanup_if_fail_with_rc(lasso_base64_decode(artifact_b64, &artifact, &artifact_len), LASSO_PROFILE_ERROR_INVALID_ARTIFACT); - if (artifact[0] != 0 || artifact[1] != 4) { /* wrong type code */ - lasso_release_string(artifact_b64); - return LASSO_PROFILE_ERROR_INVALID_ARTIFACT; - } + goto_cleanup_if_fail_with_rc(artifact_len == 44, LASSO_PROFILE_ERROR_INVALID_ARTIFACT); + + /* wrong type code */ + goto_cleanup_if_fail_with_rc(artifact[0] == 0 && artifact[1] == 4, LASSO_PROFILE_ERROR_INVALID_ARTIFACT); memcpy(provider_succinct_id, artifact+4, 20); provider_succinct_id[20] = 0; @@ -330,10 +327,7 @@ lasso_saml20_profile_init_artifact_resolve(LassoProfile *profile, lasso_assign_new_string(profile->remote_providerID, lasso_server_get_providerID_from_hash( profile->server, (char*)provider_succinct_id_b64)); - lasso_release_xml_string(provider_succinct_id_b64); - if (profile->remote_providerID == NULL) { - return LASSO_SERVER_ERROR_PROVIDER_NOT_FOUND; - } + goto_cleanup_if_fail_with_rc(profile->remote_providerID, LASSO_SERVER_ERROR_PROVIDER_NOT_FOUND); /* resolve the resolver url using the endpoint index in the artifact string */ remote_provider = lasso_server_get_provider(profile->server, profile->remote_providerID); @@ -342,15 +336,11 @@ lasso_saml20_profile_init_artifact_resolve(LassoProfile *profile, remote_provider_role, LASSO_SAML2_METADATA_ELEMENT_ARTIFACT_RESOLUTION_SERVICE, NULL, FALSE, FALSE, index_endpoint)); - if (! profile->msg_url) { - debug("looking for index endpoint %d", index_endpoint); - return LASSO_PROFILE_ERROR_ENDPOINT_INDEX_NOT_FOUND; - } - + goto_cleanup_if_fail_with_rc(profile->msg_url, LASSO_PROFILE_ERROR_ENDPOINT_INDEX_NOT_FOUND); lasso_assign_new_gobject(profile->request, lasso_samlp2_artifact_resolve_new()); request = LASSO_SAMLP2_REQUEST_ABSTRACT(profile->request); - lasso_assign_new_string(LASSO_SAMLP2_ARTIFACT_RESOLVE(request)->Artifact, artifact_b64); + lasso_transfer_string(LASSO_SAMLP2_ARTIFACT_RESOLVE(request)->Artifact, artifact_b64); request->ID = lasso_build_unique_id(32); lasso_assign_string(request->Version, "2.0"); request->Issuer = LASSO_SAML2_NAME_ID(lasso_saml2_name_id_new_with_string( @@ -361,6 +351,9 @@ lasso_saml20_profile_init_artifact_resolve(LassoProfile *profile, (LassoNode*)request)); cleanup: + lasso_release_string(artifact_b64); + lasso_release_string(artifact); + lasso_release_xml_string(provider_succinct_id_b64); return rc; } diff --git a/lasso/xml/tools.c b/lasso/xml/tools.c index 35f9ce38..bd2bf854 100644 --- a/lasso/xml/tools.c +++ b/lasso/xml/tools.c @@ -1409,36 +1409,32 @@ lasso_inflate(unsigned char *input, size_t len, size_t *outlen) gboolean lasso_node_init_from_deflated_query_part(LassoNode *node, char *deflate_string) { - int len; - xmlChar *b64_zre, *zre, *re; - xmlDoc *doc; - xmlNode *root; - - b64_zre = (xmlChar*)xmlURIUnescapeString(deflate_string, 0, NULL); - len = strlen((char*)b64_zre); - zre = xmlMalloc(len*4); - len = xmlSecBase64Decode(b64_zre, zre, len*4); - xmlFree(b64_zre); - if (len == -1) { - message(G_LOG_LEVEL_CRITICAL, "Failed to base64-decode query"); - xmlFree(zre); - return FALSE; - } + xmlChar *buffer= NULL; + char *decoded = NULL; + int decoded_len = 0; + xmlChar *re = NULL; + size_t re_len = 0; + xmlDoc *doc = NULL; + xmlNode *root = NULL; + gboolean rc = TRUE; - re = lasso_inflate(zre, len); - xmlFree(zre); + buffer = (xmlChar*)xmlURIUnescapeString(deflate_string, 0, NULL); + goto_cleanup_if_fail_with_rc(lasso_base64_decode((char*)buffer, &decoded, &decoded_len), FALSE); - if (! re) - return FALSE; + re = lasso_inflate((unsigned char*)decoded, decoded_len, &re_len); + goto_cleanup_if_fail_with_rc_with_warning(re != NULL, FALSE); doc = lasso_xml_parse_memory((char*)re, strlen((char*)re)); - lasso_release_string(re); + goto_cleanup_if_fail_with_rc_with_warning(doc != NULL, FALSE); root = xmlDocGetRootElement(doc); lasso_node_init_from_xml(node, root); +cleanup: + lasso_release_xml_string(buffer); + lasso_release_string(decoded); + lasso_release_string(re); lasso_release_doc(doc); - - return TRUE; + return rc; } char* @@ -1899,40 +1895,44 @@ lasso_xml_get_soap_content(xmlNode *root) LassoMessageFormat lasso_xml_parse_message(const char *message, LassoMessageFormat constraint, xmlDoc **doc_out, xmlNode **root_out) { - char *msg = NULL; - gboolean b64 = FALSE; + const char *msg = NULL; + int msg_len = 0; + char *base64_decoded_message = NULL; LassoMessageFormat rc = LASSO_MESSAGE_FORMAT_UNKNOWN; xmlDoc *doc = NULL; xmlNode *root = NULL; gboolean any = constraint == LASSO_MESSAGE_FORMAT_UNKNOWN; - msg = (char*)message; - /* BASE64 case */ if (any || constraint == LASSO_MESSAGE_FORMAT_BASE64) { if (message[0] != 0 && is_base64(message)) { - msg = g_malloc(strlen(message)); - rc = xmlSecBase64Decode((xmlChar*)message, (xmlChar*)msg, strlen(message)); - if (rc >= 0) { - b64 = TRUE; - } else { - lasso_release(msg); - msg = (char*)message; + if (lasso_base64_decode(message, &base64_decoded_message, &msg_len)) { + rc = LASSO_MESSAGE_FORMAT_BASE64; + msg = base64_decoded_message; } } } + if (! msg) { + msg = message; + msg_len = strlen(message); + } + /* XML case */ if (any || constraint == LASSO_MESSAGE_FORMAT_BASE64 || constraint == LASSO_MESSAGE_FORMAT_XML || constraint == LASSO_MESSAGE_FORMAT_SOAP) { if (strchr(msg, '<')) { - doc = lasso_xml_parse_memory(msg, strlen(msg)); + doc = lasso_xml_parse_memory(msg, msg_len); if (doc == NULL) { rc = LASSO_MESSAGE_FORMAT_UNKNOWN; goto cleanup; } root = xmlDocGetRootElement(doc); + if (! root) { + rc = LASSO_MESSAGE_FORMAT_ERROR; + goto cleanup; + } if (any || constraint == LASSO_MESSAGE_FORMAT_SOAP) { gboolean is_soap = FALSE; @@ -1941,26 +1941,20 @@ lasso_xml_parse_message(const char *message, LassoMessageFormat constraint, xmlD if (is_soap) { root = lasso_xml_get_soap_content(root); } - if (! root) { - rc = LASSO_MESSAGE_FORMAT_ERROR; - goto cleanup; - } if (is_soap) { rc = LASSO_MESSAGE_FORMAT_SOAP; goto cleanup; } - if (b64) { - lasso_release(msg); - rc = LASSO_MESSAGE_FORMAT_BASE64; + if (rc == LASSO_MESSAGE_FORMAT_BASE64) { goto cleanup; } rc = LASSO_MESSAGE_FORMAT_XML; - goto cleanup; } } } cleanup: + lasso_release(base64_decoded_message); if (doc_out) { *doc_out = doc; if (root_out) { @@ -1968,7 +1962,6 @@ cleanup: } } else { lasso_release_doc(doc); - lasso_release_xml_node(root); } return rc; } @@ -2672,36 +2665,30 @@ lasso_xmlsec_load_private_key(const char *filename_or_buffer, const char *passwo gboolean lasso_get_base64_content(xmlNode *node, char **content, size_t *length) { - xmlChar *base64, *stripped_base64; - xmlChar *result; - int base64_length; - int rc = 0; + xmlChar *base64 = NULL; + xmlChar *stripped_base64 = NULL; + char *decoded = NULL; + int decoded_length = 0; + int rc = TRUE; - if (! node || ! content || ! length) - return FALSE; + goto_cleanup_if_fail_with_rc(node && content && length, FALSE); base64 = xmlNodeGetContent(node); - if (! base64) - return FALSE; - stripped_base64 = base64; + goto_cleanup_if_fail_with_rc(base64, FALSE); + /* skip spaces */ + stripped_base64 = base64; while (*stripped_base64 && isspace(*stripped_base64)) stripped_base64++; - base64_length = strlen((char*)stripped_base64); - result = g_new(xmlChar, base64_length); - xmlSecErrorsDefaultCallbackEnableOutput(FALSE); - rc = xmlSecBase64Decode(stripped_base64, result, base64_length); - xmlSecErrorsDefaultCallbackEnableOutput(TRUE); - xmlFree(base64); - if (rc < 0) { - return FALSE; - } else { - *content = (char*)g_memdup(result, rc); - xmlFree(result); - *length = rc; - return TRUE; - } + goto_cleanup_if_fail_with_rc(lasso_base64_decode((char*)stripped_base64, &decoded, &decoded_length), FALSE); + lasso_transfer_string(*content, decoded); + *length = decoded_length; +cleanup: + lasso_release_xml_string(base64); + lasso_release_string(decoded); + return rc; + } xmlSecKeyPtr @@ -3198,13 +3185,13 @@ static char* lasso_get_saml_message(xmlChar **query_fields) { int i = 0; char *enc = NULL; - char *message = NULL; + char *raw_message = NULL; + char *gziped_message = NULL; + int gziped_message_len = 0; char *saml_message = NULL; - char *decoded_message = NULL; + size_t saml_message_len = 0; xmlChar *field = NULL; char *t = NULL; - int rc = 0; - int len = 0; for (i=0; (field=query_fields[i]); i++) { t = strchr((char*)field, '='); @@ -3216,11 +3203,11 @@ lasso_get_saml_message(xmlChar **query_fields) { continue; } if (strcmp((char*)field, LASSO_SAML2_FIELD_REQUEST) == 0 || strcmp((char*)field, LASSO_SAML2_FIELD_RESPONSE) == 0) { - message = t+1; + raw_message = t+1; continue; } } - if (message == NULL) { + if (raw_message == NULL) { return NULL; } if (enc && strcmp(enc, LASSO_SAML2_DEFLATE_ENCODING) != 0) { @@ -3228,23 +3215,12 @@ lasso_get_saml_message(xmlChar **query_fields) { debug("Unknown URL encoding: %64s", enc); return NULL; } - len = strlen(message); - decoded_message = g_malloc(len); - if (! is_base64(message)) { - debug("message is not base64"); - goto cleanup; - } - rc = xmlSecBase64Decode((xmlChar*)message, (xmlChar*)decoded_message, len); - if (rc < 0) { - debug("could not decode redirect SAML message"); - goto cleanup; - } - /* rc contains the length of the result */ - saml_message = (char*)lasso_inflate((unsigned char*) decoded_message, rc); + + goto_cleanup_if_fail(lasso_base64_decode(raw_message, &gziped_message, &gziped_message_len)) + saml_message = (char*)lasso_inflate((unsigned char*)gziped_message, gziped_message_len, &saml_message_len); cleanup: - if (decoded_message) { - lasso_release(decoded_message); - } + lasso_release_string(gziped_message); + return saml_message; } @@ -3260,6 +3236,7 @@ lasso_xmltextreader_from_message(const char *message, char **to_free) { char *needle; xmlChar **query_fields = NULL; char *decoded_message = NULL; + int decoded_message_len = 0; xmlTextReader *reader = NULL; g_assert(to_free); @@ -3275,22 +3252,17 @@ lasso_xmltextreader_from_message(const char *message, char **to_free) { } len = strlen(message); } else { /* POST */ - int rc = 0; - if (! is_base64(message)) { debug("POST message is not base64"); goto cleanup; } - decoded_message = g_malloc(len); - rc = xmlSecBase64Decode((xmlChar*)message, (xmlChar*)decoded_message, len); - if (rc < 0) { + if (! lasso_base64_decode(message, &decoded_message, &decoded_message_len)) { debug("could not decode POST SAML message"); goto cleanup; } - len = rc; - decoded_message[len] = '\0'; - message = *to_free = decoded_message; - decoded_message = NULL; + message = decoded_message; + len = decoded_message_len; + lasso_transfer_string(*to_free, decoded_message); } } @@ -3298,9 +3270,7 @@ lasso_xmltextreader_from_message(const char *message, char **to_free) { reader = xmlReaderForMemory(message, len, "", NULL, XML_PARSE_NONET); cleanup: - if (query_fields) - lasso_release_array_of_xml_strings(query_fields); - if (decoded_message) - lasso_release_string(decoded_message); + lasso_release_array_of_xml_strings(query_fields); + lasso_release_string(decoded_message); return reader; } diff --git a/lasso/xml/xml.c b/lasso/xml/xml.c index a921b2c4..0d5c6e31 100644 --- a/lasso/xml/xml.c +++ b/lasso/xml/xml.c @@ -2534,7 +2534,9 @@ is_base64(const char *message) LassoMessageFormat lasso_node_init_from_message_with_format(LassoNode *node, const char *message, LassoMessageFormat constraint, xmlDoc **doc_out, xmlNode **root_out) { - char *msg = NULL; + char *decoded_msg = NULL; + int decoded_msg_len = 0; + const char *msg = NULL; gboolean b64 = FALSE; LassoMessageFormat rc = LASSO_MESSAGE_FORMAT_ERROR; xmlDoc *doc = NULL; @@ -2546,15 +2548,11 @@ lasso_node_init_from_message_with_format(LassoNode *node, const char *message, L /* BASE64 case */ if (any || constraint == LASSO_MESSAGE_FORMAT_BASE64) { if (message[0] != 0 && is_base64(message)) { - int rc = 0; - - msg = g_malloc(strlen(message)); - rc = xmlSecBase64Decode((xmlChar*)message, (xmlChar*)msg, strlen(message)); - if (rc >= 0) { + if (lasso_base64_decode(message, &decoded_msg, &decoded_msg_len)) { b64 = TRUE; + msg = decoded_msg; } else { - lasso_release(msg); - msg = (char*)message; + msg = message; } } } @@ -2589,7 +2587,6 @@ lasso_node_init_from_message_with_format(LassoNode *node, const char *message, L goto cleanup; } if (b64) { - lasso_release(msg); rc = LASSO_MESSAGE_FORMAT_BASE64; goto cleanup; } @@ -2612,6 +2609,7 @@ lasso_node_init_from_message_with_format(LassoNode *node, const char *message, L } cleanup: + lasso_release_string(decoded_msg); if (doc_out) { *doc_out = doc; if (root_out) { -- 2.37.2