krb5 commit: Improve argument validation in some GSS APIs
Greg Hudson
ghudson at mit.edu
Mon Oct 21 20:47:22 EDT 2019
https://github.com/krb5/krb5/commit/b835476dba949bad41295ce13afa7e9270963c20
commit b835476dba949bad41295ce13afa7e9270963c20
Author: Greg Hudson <ghudson at mit.edu>
Date: Thu Oct 17 00:52:04 2019 -0400
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.
src/lib/gssapi/generic/gssapi_generic.c | 4 ++++
src/lib/gssapi/mechglue/g_authorize_localname.c | 3 +--
src/lib/gssapi/mechglue/g_complete_auth_token.c | 5 +++++
src/lib/gssapi/mechglue/g_del_name_attr.c | 3 +--
src/lib/gssapi/mechglue/g_export_name_comp.c | 10 ++++++++--
src/lib/gssapi/mechglue/g_get_name_attr.c | 22 +++++++++++-----------
src/lib/gssapi/mechglue/g_initialize.c | 3 +++
src/lib/gssapi/mechglue/g_inq_context_oid.c | 8 +++++++-
src/lib/gssapi/mechglue/g_inq_cred_oid.c | 12 +++++++++---
src/lib/gssapi/mechglue/g_inq_name.c | 14 ++++++++------
src/lib/gssapi/mechglue/g_map_name_to_any.c | 13 +++++++------
src/lib/gssapi/mechglue/g_mechattr.c | 20 ++++++++++----------
src/lib/gssapi/mechglue/g_prf.c | 12 ++++++++++--
src/lib/gssapi/mechglue/g_rel_name_mapping.c | 3 +--
src/lib/gssapi/mechglue/g_saslname.c | 9 +++++----
src/lib/gssapi/mechglue/g_set_context_option.c | 3 +--
src/lib/gssapi/mechglue/g_set_cred_option.c | 3 +--
src/lib/gssapi/mechglue/g_set_name_attr.c | 3 +--
src/lib/gssapi/mechglue/g_set_neg_mechs.c | 3 +--
19 files changed, 94 insertions(+), 59 deletions(-)
diff --git a/src/lib/gssapi/generic/gssapi_generic.c b/src/lib/gssapi/generic/gssapi_generic.c
index fa144c2..1b362c3 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 0e4fb07..b6fc54d 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 4bcb47e..4f028a7 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 e81e331..21f1568 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 ab538a0..0a2bac4 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 047d5d4..2108beb 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 0054acf..120d73e 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 ebdeaae..d375cfc 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 df51b44..6d8594d 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 60a3b54..cd1cbe5 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 ebf4945..0e5490b 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 e49651e..5d3e3f1 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 9e168ad..96f2fac 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 f09136a..0363178 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 48060c3..e25f9e0 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 87db240..8e25a27 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 90e5756..5c9d21d 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 a479762..42bde49 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 69cac70..9b04ec9 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;
More information about the cvs-krb5
mailing list