Skip to content

Commit

Permalink
Improve argument validation in some GSS APIs
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@greghudson
greghudson committed Oct 21, 2019
1 parent d09215c commit b835476
Show file tree
Hide file tree
Showing 19 changed files with 94 additions and 59 deletions.
4 changes: 4 additions & 0 deletions src/lib/gssapi/generic/gssapi_generic.c
View file
Expand Up @@ -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;
Expand All @@ -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];

Expand Down
3 changes: 1 addition & 2 deletions src/lib/gssapi/mechglue/g_authorize_localname.c
View file
Expand Up @@ -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;

Expand Down
5 changes: 5 additions & 0 deletions src/lib/gssapi/mechglue/g_complete_auth_token.c
View file
Expand Up @@ -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;

Expand Down
3 changes: 1 addition & 2 deletions src/lib/gssapi/mechglue/g_del_name_attr.c
View file
Expand Up @@ -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)
Expand Down
10 changes: 8 additions & 2 deletions src/lib/gssapi/mechglue/g_export_name_comp.c
View file
Expand Up @@ -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;

Expand All @@ -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)
Expand Down
22 changes: 11 additions & 11 deletions src/lib/gssapi/mechglue/g_get_name_attr.c
View file
Expand Up @@ -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)
Expand All @@ -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;

Expand Down
3 changes: 3 additions & 0 deletions src/lib/gssapi/mechglue/g_initialize.c
View file
Expand Up @@ -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);

Expand Down
8 changes: 7 additions & 1 deletion src/lib/gssapi/mechglue/g_inq_context_oid.c
View file
Expand Up @@ -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)
Expand Down
12 changes: 9 additions & 3 deletions src/lib/gssapi/mechglue/g_inq_cred_oid.c
View file
Expand Up @@ -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;

Expand Down
14 changes: 8 additions & 6 deletions src/lib/gssapi/mechglue/g_inq_name.c
View file
Expand Up @@ -38,19 +38,21 @@ 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;

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) {
Expand Down
13 changes: 7 additions & 6 deletions src/lib/gssapi/mechglue/g_map_name_to_any.c
View file
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
20 changes: 10 additions & 10 deletions src/lib/gssapi/mechglue/g_mechattr.c
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -163,17 +162,18 @@ 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;

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;
Expand Down
12 changes: 10 additions & 2 deletions src/lib/gssapi/mechglue/g_prf.c
View file
Expand Up @@ -38,17 +38,25 @@ 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;

if (context_handle == GSS_C_NO_CONTEXT)
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;
Expand Down
3 changes: 1 addition & 2 deletions src/lib/gssapi/mechglue/g_rel_name_mapping.c
View file
Expand Up @@ -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;
Expand All @@ -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)
Expand Down
9 changes: 5 additions & 4 deletions src/lib/gssapi/mechglue/g_saslname.c
View file
Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions src/lib/gssapi/mechglue/g_set_context_option.c
View file
Expand Up @@ -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.
Expand Down
3 changes: 1 addition & 2 deletions src/lib/gssapi/mechglue/g_set_cred_option.c
View file
Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions src/lib/gssapi/mechglue/g_set_name_attr.c
View file
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions src/lib/gssapi/mechglue/g_set_neg_mechs.c
View file
Expand Up @@ -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;
Expand Down

0 comments on commit b835476

Please sign in to comment.