Re: [PATCH 1/2] _gsasl_gssapi_server_step: don't overwrite maj_stat

[Simon Josefsson]

Hi Andreas.  Thank you!  The code definitely looks buggy.  I'm not
convinced this is exploitable though since GSSAPI means Kerberos V5 and
the code will in the next steps unwrap/wrap a token which will fail
unless the context hasn't been negotiated successfully.  Do you agree,
or do you have some other analysis?

Your patch makes a direct comparison against GSS_S_COMPLETE which is
poor style in general, and using GSS_ERROR is preferred.  How about the
following patch instead?

/Simon

diff --git a/lib/gssapi/server.c b/lib/gssapi/server.c
index dc05a6f..2cedc2a 100644
--- a/lib/gssapi/server.c
+++ b/lib/gssapi/server.c
@@ -168,13 +168,13 @@ _gsasl_gssapi_server_step (Gsasl_session * sctx,
       memcpy (*output, bufdesc2.value, bufdesc2.length);
       *output_len = bufdesc2.length;
 
+      if (maj_stat == GSS_S_COMPLETE)
+       state->step++;
+
       maj_stat = gss_release_buffer (&min_stat, &bufdesc2);
       if (GSS_ERROR (maj_stat))
        return GSASL_GSSAPI_RELEASE_BUFFER_ERROR;
 
-      if (maj_stat == GSS_S_COMPLETE)
-       state->step++;
-
       res = GSASL_NEEDS_MORE;
       break;