From: Greg Hudson Date: Thu, 17 Oct 2019 04:52:04 +0000 (-0400) Subject: Improve argument validation in some GSS APIs X-Git-Tag: krb5-1.18-beta1~45 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F988%2Fhead;p=thirdparty%2Fkrb5.git Improve argument validation in some GSS APIs The prevailing discpline of public GSS APIs is to set output parameters to default values, then validate input parameters. Some more recent APIs did not do this consistently, leading to the possibility of minor_status retaining its previous value or similar issues. --- diff --git a/src/lib/gssapi/generic/gssapi_generic.c b/src/lib/gssapi/generic/gssapi_generic.c index fa144c2bf9..1b362c3d8b 100644 --- a/src/lib/gssapi/generic/gssapi_generic.c +++ b/src/lib/gssapi/generic/gssapi_generic.c @@ -413,6 +413,8 @@ generic_gss_display_mech_attr( { size_t i; + if (minor_status != NULL) + *minor_status = 0; if (name != GSS_C_NO_BUFFER) { name->length = 0; name->value = NULL; @@ -425,6 +427,8 @@ generic_gss_display_mech_attr( long_desc->length = 0; long_desc->value = NULL; } + if (minor_status == NULL) + return GSS_S_CALL_INACCESSIBLE_WRITE; for (i = 0; i < sizeof(mech_attr_info)/sizeof(mech_attr_info[0]); i++) { struct mech_attr_info_desc *mai = &mech_attr_info[i]; diff --git a/src/lib/gssapi/mechglue/g_authorize_localname.c b/src/lib/gssapi/mechglue/g_authorize_localname.c index 0e4fb07229..b6fc54da8d 100644 --- a/src/lib/gssapi/mechglue/g_authorize_localname.c +++ b/src/lib/gssapi/mechglue/g_authorize_localname.c @@ -169,12 +169,11 @@ gss_authorize_localname(OM_uint32 *minor, if (minor == NULL) return (GSS_S_CALL_INACCESSIBLE_WRITE); + *minor = 0; if (name == GSS_C_NO_NAME || user == GSS_C_NO_NAME) return (GSS_S_CALL_INACCESSIBLE_READ); - *minor = 0; - unionName = (gss_union_name_t)name; unionUser = (gss_union_name_t)user; diff --git a/src/lib/gssapi/mechglue/g_complete_auth_token.c b/src/lib/gssapi/mechglue/g_complete_auth_token.c index 4bcb47e84b..4f028a77ee 100644 --- a/src/lib/gssapi/mechglue/g_complete_auth_token.c +++ b/src/lib/gssapi/mechglue/g_complete_auth_token.c @@ -43,6 +43,11 @@ gss_complete_auth_token (OM_uint32 *minor_status, gss_union_ctx_id_t ctx; gss_mechanism mech; + if (minor_status == NULL) + return GSS_S_CALL_INACCESSIBLE_WRITE; + *minor_status = 0; + if (input_message_buffer == GSS_C_NO_BUFFER) + return GSS_S_CALL_INACCESSIBLE_READ; if (context_handle == GSS_C_NO_CONTEXT) return GSS_S_NO_CONTEXT; diff --git a/src/lib/gssapi/mechglue/g_del_name_attr.c b/src/lib/gssapi/mechglue/g_del_name_attr.c index e81e3315ad..21f156813e 100644 --- a/src/lib/gssapi/mechglue/g_del_name_attr.c +++ b/src/lib/gssapi/mechglue/g_del_name_attr.c @@ -38,12 +38,11 @@ gss_delete_name_attribute(OM_uint32 *minor_status, if (minor_status == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; + *minor_status = 0; if (name == GSS_C_NO_NAME) return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_BAD_NAME; - *minor_status = 0; - union_name = (gss_union_name_t)name; if (union_name->mech_type == GSS_C_NO_OID) diff --git a/src/lib/gssapi/mechglue/g_export_name_comp.c b/src/lib/gssapi/mechglue/g_export_name_comp.c index ab538a095d..0a2bac41fb 100644 --- a/src/lib/gssapi/mechglue/g_export_name_comp.c +++ b/src/lib/gssapi/mechglue/g_export_name_comp.c @@ -39,6 +39,14 @@ gss_export_name_composite(OM_uint32 *minor_status, gss_union_name_t union_name; gss_mechanism mech; + if (minor_status != NULL) + *minor_status = 0; + + if (exp_composite_name != GSS_C_NO_BUFFER) { + exp_composite_name->value = NULL; + exp_composite_name->length = 0; + } + if (minor_status == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; @@ -48,8 +56,6 @@ gss_export_name_composite(OM_uint32 *minor_status, if (exp_composite_name == GSS_C_NO_BUFFER) return GSS_S_CALL_INACCESSIBLE_WRITE; - *minor_status = 0; - union_name = (gss_union_name_t)name; if (union_name->mech_type == GSS_C_NO_OID) diff --git a/src/lib/gssapi/mechglue/g_get_name_attr.c b/src/lib/gssapi/mechglue/g_get_name_attr.c index 047d5d428f..2108beb361 100644 --- a/src/lib/gssapi/mechglue/g_get_name_attr.c +++ b/src/lib/gssapi/mechglue/g_get_name_attr.c @@ -41,16 +41,8 @@ gss_get_name_attribute(OM_uint32 *minor_status, gss_union_name_t union_name; gss_mechanism mech; - if (minor_status == NULL) - return GSS_S_CALL_INACCESSIBLE_WRITE; - - if (name == GSS_C_NO_NAME) - return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_BAD_NAME; - if (attr == GSS_C_NO_BUFFER) - return GSS_S_CALL_INACCESSIBLE_READ; - if (more == NULL) - return GSS_S_CALL_INACCESSIBLE_WRITE; - + if (minor_status != NULL) + *minor_status = 0; if (authenticated != NULL) *authenticated = 0; if (complete != NULL) @@ -64,7 +56,15 @@ gss_get_name_attribute(OM_uint32 *minor_status, display_value->length = 0; } - *minor_status = 0; + if (minor_status == NULL) + return GSS_S_CALL_INACCESSIBLE_WRITE; + + if (name == GSS_C_NO_NAME) + return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_BAD_NAME; + if (attr == GSS_C_NO_BUFFER) + return GSS_S_CALL_INACCESSIBLE_READ; + if (more == NULL) + return GSS_S_CALL_INACCESSIBLE_WRITE; union_name = (gss_union_name_t)name; diff --git a/src/lib/gssapi/mechglue/g_initialize.c b/src/lib/gssapi/mechglue/g_initialize.c index 0054acf88e..120d73e2f2 100644 --- a/src/lib/gssapi/mechglue/g_initialize.c +++ b/src/lib/gssapi/mechglue/g_initialize.c @@ -168,6 +168,9 @@ gss_OID *oid; OM_uint32 major; gss_mech_info aMech; + if (minor_status != NULL) + *minor_status = 0; + if (minor_status == NULL || oid == NULL) return (GSS_S_CALL_INACCESSIBLE_WRITE); diff --git a/src/lib/gssapi/mechglue/g_inq_context_oid.c b/src/lib/gssapi/mechglue/g_inq_context_oid.c index ebdeaaee88..d375cfc795 100644 --- a/src/lib/gssapi/mechglue/g_inq_context_oid.c +++ b/src/lib/gssapi/mechglue/g_inq_context_oid.c @@ -36,7 +36,13 @@ gss_inquire_sec_context_by_oid (OM_uint32 *minor_status, gss_union_ctx_id_t ctx; gss_mechanism mech; - if (minor_status == NULL) + if (minor_status != NULL) + *minor_status = 0; + + if (data_set != NULL) + *data_set = GSS_C_NO_BUFFER_SET; + + if (minor_status == NULL || data_set == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; if (context_handle == GSS_C_NO_CONTEXT) diff --git a/src/lib/gssapi/mechglue/g_inq_cred_oid.c b/src/lib/gssapi/mechglue/g_inq_cred_oid.c index df51b44e9a..6d8594d1bb 100644 --- a/src/lib/gssapi/mechglue/g_inq_cred_oid.c +++ b/src/lib/gssapi/mechglue/g_inq_cred_oid.c @@ -74,14 +74,20 @@ gss_inquire_cred_by_oid(OM_uint32 *minor_status, gss_buffer_set_t ret_set = GSS_C_NO_BUFFER_SET; OM_uint32 status, minor; - if (minor_status == NULL) + if (minor_status != NULL) + *minor_status = 0; + + if (data_set != NULL) + *data_set = GSS_C_NO_BUFFER_SET; + + if (minor_status == NULL || data_set == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; if (cred_handle == GSS_C_NO_CREDENTIAL) return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_NO_CRED; - *minor_status = 0; - *data_set = GSS_C_NO_BUFFER_SET; + if (desired_object == GSS_C_NO_OID) + return GSS_S_CALL_INACCESSIBLE_READ; union_cred = (gss_union_cred_t) cred_handle; diff --git a/src/lib/gssapi/mechglue/g_inq_name.c b/src/lib/gssapi/mechglue/g_inq_name.c index 60a3b54e79..cd1cbe5d90 100644 --- a/src/lib/gssapi/mechglue/g_inq_name.c +++ b/src/lib/gssapi/mechglue/g_inq_name.c @@ -38,11 +38,8 @@ gss_inquire_name(OM_uint32 *minor_status, gss_union_name_t union_name; gss_mechanism mech; - if (minor_status == NULL) - return GSS_S_CALL_INACCESSIBLE_WRITE; - - if (name == GSS_C_NO_NAME) - return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_BAD_NAME; + if (minor_status != NULL) + *minor_status = 0; if (MN_mech != NULL) *MN_mech = GSS_C_NO_OID; @@ -50,7 +47,12 @@ gss_inquire_name(OM_uint32 *minor_status, if (attrs != NULL) *attrs = GSS_C_NO_BUFFER_SET; - *minor_status = 0; + if (minor_status == NULL) + return GSS_S_CALL_INACCESSIBLE_WRITE; + + if (name == GSS_C_NO_NAME) + return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_BAD_NAME; + union_name = (gss_union_name_t)name; if (union_name->mech_type == GSS_C_NO_OID) { diff --git a/src/lib/gssapi/mechglue/g_map_name_to_any.c b/src/lib/gssapi/mechglue/g_map_name_to_any.c index ebf49450f0..0e5490bea9 100644 --- a/src/lib/gssapi/mechglue/g_map_name_to_any.c +++ b/src/lib/gssapi/mechglue/g_map_name_to_any.c @@ -38,7 +38,13 @@ gss_map_name_to_any(OM_uint32 *minor_status, gss_union_name_t union_name; gss_mechanism mech; - if (minor_status == NULL) + if (minor_status != NULL) + *minor_status = 0; + + if (output != NULL) + *output = NULL; + + if (minor_status == NULL || output == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; if (name == GSS_C_NO_NAME) @@ -47,11 +53,6 @@ gss_map_name_to_any(OM_uint32 *minor_status, if (type_id == GSS_C_NO_BUFFER) return GSS_S_CALL_INACCESSIBLE_READ; - if (output == NULL) - return GSS_S_CALL_INACCESSIBLE_WRITE; - - *minor_status = 0; - union_name = (gss_union_name_t)name; if (union_name->mech_type == GSS_C_NO_OID) diff --git a/src/lib/gssapi/mechglue/g_mechattr.c b/src/lib/gssapi/mechglue/g_mechattr.c index e49651eb6a..5d3e3f18ce 100644 --- a/src/lib/gssapi/mechglue/g_mechattr.c +++ b/src/lib/gssapi/mechglue/g_mechattr.c @@ -100,16 +100,15 @@ gss_indicate_mechs_by_attrs( gss_OID_set allMechs = GSS_C_NO_OID_SET; size_t i; - if (minor == NULL) - return GSS_S_CALL_INACCESSIBLE_WRITE; + if (minor != NULL) + *minor = 0; - *minor = 0; + if (mechs != NULL) + *mechs = GSS_C_NO_OID_SET; - if (mechs == NULL) + if (minor == NULL || mechs == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; - *mechs = GSS_C_NO_OID_SET; - status = gss_indicate_mechs(minor, &allMechs); if (GSS_ERROR(status)) goto cleanup; @@ -163,10 +162,8 @@ gss_inquire_attrs_for_mech( gss_OID selected_mech, public_mech; gss_mechanism mech; - if (minor == NULL) - return GSS_S_CALL_INACCESSIBLE_WRITE; - - *minor = 0; + if (minor != NULL) + *minor = 0; if (mech_attrs != NULL) *mech_attrs = GSS_C_NO_OID_SET; @@ -174,6 +171,9 @@ gss_inquire_attrs_for_mech( if (known_mech_attrs != NULL) *known_mech_attrs = GSS_C_NO_OID_SET; + if (minor == NULL) + return GSS_S_CALL_INACCESSIBLE_WRITE; + status = gssint_select_mech_type(minor, mech_oid, &selected_mech); if (status != GSS_S_COMPLETE) return status; diff --git a/src/lib/gssapi/mechglue/g_prf.c b/src/lib/gssapi/mechglue/g_prf.c index 9e168adfe0..96f2facf89 100644 --- a/src/lib/gssapi/mechglue/g_prf.c +++ b/src/lib/gssapi/mechglue/g_prf.c @@ -38,6 +38,14 @@ gss_pseudo_random (OM_uint32 *minor_status, gss_union_ctx_id_t ctx; gss_mechanism mech; + if (minor_status != NULL) + *minor_status = 0; + + if (prf_out != GSS_C_NO_BUFFER) { + prf_out->length = 0; + prf_out->value = NULL; + } + if (minor_status == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; @@ -45,10 +53,10 @@ gss_pseudo_random (OM_uint32 *minor_status, return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_NO_CONTEXT; if (prf_in == GSS_C_NO_BUFFER) - return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_NO_CONTEXT; + return GSS_S_CALL_INACCESSIBLE_READ; if (prf_out == GSS_C_NO_BUFFER) - return GSS_S_CALL_INACCESSIBLE_WRITE | GSS_S_NO_CONTEXT; + return GSS_S_CALL_INACCESSIBLE_WRITE; prf_out->length = 0; prf_out->value = NULL; diff --git a/src/lib/gssapi/mechglue/g_rel_name_mapping.c b/src/lib/gssapi/mechglue/g_rel_name_mapping.c index f09136afee..0363178273 100644 --- a/src/lib/gssapi/mechglue/g_rel_name_mapping.c +++ b/src/lib/gssapi/mechglue/g_rel_name_mapping.c @@ -39,6 +39,7 @@ gss_release_any_name_mapping(OM_uint32 *minor_status, if (minor_status == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; + *minor_status = 0; if (name == GSS_C_NO_NAME) return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_BAD_NAME; @@ -49,8 +50,6 @@ gss_release_any_name_mapping(OM_uint32 *minor_status, if (input == NULL) return GSS_S_CALL_INACCESSIBLE_READ; - *minor_status = 0; - union_name = (gss_union_name_t)name; if (union_name->mech_type == GSS_C_NO_OID) diff --git a/src/lib/gssapi/mechglue/g_saslname.c b/src/lib/gssapi/mechglue/g_saslname.c index 48060c3671..e25f9e0a53 100644 --- a/src/lib/gssapi/mechglue/g_saslname.c +++ b/src/lib/gssapi/mechglue/g_saslname.c @@ -177,14 +177,15 @@ OM_uint32 KRB5_CALLCONV gss_inquire_mech_for_saslname( gss_OID_set mechSet = GSS_C_NO_OID_SET; size_t i; - if (minor_status == NULL) - return GSS_S_CALL_INACCESSIBLE_WRITE; - - *minor_status = 0; + if (minor_status != NULL) + *minor_status = 0; if (mech_type != NULL) *mech_type = GSS_C_NO_OID; + if (minor_status == NULL) + return GSS_S_CALL_INACCESSIBLE_WRITE; + status = gss_indicate_mechs(minor_status, &mechSet); if (status != GSS_S_COMPLETE) return status; diff --git a/src/lib/gssapi/mechglue/g_set_context_option.c b/src/lib/gssapi/mechglue/g_set_context_option.c index 87db240df3..8e25a277f8 100644 --- a/src/lib/gssapi/mechglue/g_set_context_option.c +++ b/src/lib/gssapi/mechglue/g_set_context_option.c @@ -44,12 +44,11 @@ gss_set_sec_context_option (OM_uint32 *minor_status, if (minor_status == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; + *minor_status = 0; if (context_handle == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; - *minor_status = 0; - /* * select the approprate underlying mechanism routine and * call it. diff --git a/src/lib/gssapi/mechglue/g_set_cred_option.c b/src/lib/gssapi/mechglue/g_set_cred_option.c index 90e5756e51..5c9d21dbc1 100644 --- a/src/lib/gssapi/mechglue/g_set_cred_option.c +++ b/src/lib/gssapi/mechglue/g_set_cred_option.c @@ -103,12 +103,11 @@ gss_set_cred_option(OM_uint32 *minor_status, if (minor_status == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; + *minor_status = 0; if (cred_handle == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; - *minor_status = 0; - status = GSS_S_UNAVAILABLE; if (*cred_handle == GSS_C_NO_CREDENTIAL) { diff --git a/src/lib/gssapi/mechglue/g_set_name_attr.c b/src/lib/gssapi/mechglue/g_set_name_attr.c index a479762a76..42bde490c5 100644 --- a/src/lib/gssapi/mechglue/g_set_name_attr.c +++ b/src/lib/gssapi/mechglue/g_set_name_attr.c @@ -40,12 +40,11 @@ gss_set_name_attribute(OM_uint32 *minor_status, if (minor_status == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; + *minor_status = 0; if (name == GSS_C_NO_NAME) return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_BAD_NAME; - *minor_status = 0; - union_name = (gss_union_name_t)name; if (union_name->mech_type == GSS_C_NO_OID) diff --git a/src/lib/gssapi/mechglue/g_set_neg_mechs.c b/src/lib/gssapi/mechglue/g_set_neg_mechs.c index 69cac70373..9b04ec9d0e 100644 --- a/src/lib/gssapi/mechglue/g_set_neg_mechs.c +++ b/src/lib/gssapi/mechglue/g_set_neg_mechs.c @@ -37,12 +37,11 @@ gss_set_neg_mechs(OM_uint32 *minor_status, if (minor_status == NULL) return GSS_S_CALL_INACCESSIBLE_WRITE; + *minor_status = 0; if (cred_handle == GSS_C_NO_CREDENTIAL) return GSS_S_CALL_INACCESSIBLE_READ | GSS_S_NO_CRED; - *minor_status = 0; - union_cred = (gss_union_cred_t) cred_handle; avail = 0;