Infinite loop when not compiled with HAVE_GSSAPI #3518

Closed
michaelrsweet opened this Issue Mar 3, 2010 · 7 comments

1 participant

@michaelrsweet

Version: 1.4.2
CUPS.org User: bernerus

If cups is compiled without HAVE_GSSAPI and is used with a server that requires authorization, the client gets into an infinite loop.

Without HAVE_GSSAPI there is no code in auth.c that tests the
http->digest_tries variable.

The bug does not show unless the http input buffer is flushed after receiving a HTTP_UNAUTHORIZED, which is necessary to do. If that is not done, submission fails before getting into this loop.

Fix:

--- cups/auth.c (revision 9014)
+++ cups/auth.c (working copy)
@@ -392,6 +394,13 @@
httpSetAuthString(http, "Digest", digest);
}

  • if (http->status == HTTP_UNAUTHORIZED && http->digest_tries >= 3)
  • {
  • DEBUG_printf(("1cupsDoAuthentication: too many authentication tries (%d)", http->digest_tries));
  • http->status = HTTP_AUTHORIZATION_CANCELED;
  • return(-1);
  • }
    +
    DEBUG_printf(("1cupsDoAuthentication: authstring=\"%s\"", http->authstring));

    return (0);

@michaelrsweet

CUPS.org User: mike

OK, I've opted to just move the check for both regular and Kerberos above the setting of the auth string - patch attached.

@michaelrsweet

CUPS.org User: bernerus

OK. That looks even better, Thanks!

@michaelrsweet

CUPS.org User: mike

Fixed in Subversion repository.

@michaelrsweet

CUPS.org User: kurtseifried

CVE-2010-2432 has been assigned for this.

@michaelrsweet

CUPS.org User: thoger

Do you have any minimal reproducer that triggers this flaw? I presume the was requesting Negotiate authentication.

Reading the patch, I'm wondering if it does what it was intended to do. Based on the previous comments and article L596, it seems intention was to cancel even non-Negotiate authentication after 3 failures by moving "Too many authentication tries" error to a common code path. However, following precedes that check:

if ((http->digest_tries > 1 || !http->userpass[0]) &&
strncmp(http->fields[HTTP_FIELD_WWW_AUTHENTICATE], "Negotiate", 9))

which leads to password callback call for non-Negotiate authentications and reset of digest_tries counter. So instead of "Too many tries" error, there's another password prompt. Depending on the callback function, this may keep resending password to the server which replies with "unauthorized" without being cancelled as expected (?). Or was there some additional loop that did not involve active request-unauthorized network communication?

@michaelrsweet

CUPS.org User: mike

The intent was to make sure that non-password authentication was not tried too many times. When password authentication is requested we'll keep retrying until the password callback returns NULL or the server returns a "forbidden" status.

@michaelrsweet

"str3518.patch":

Index: cups/auth.c

--- cups/auth.c (revision 9016)
+++ cups/auth.c (working copy)
@@ -181,6 +181,15 @@
else if (http->status == HTTP_UNAUTHORIZED)
http->digest_tries ++;

  • if (http->status == HTTP_UNAUTHORIZED && http->digest_tries >= 3)
  • {
  • DEBUG_printf(("1cupsDoAuthentication: Too many authentication tries (%d)",
  • http->digest_tries)); +
  • http->status = HTTP_AUTHORIZATION_CANCELED;
  • return (-1);
  • }
    +
    /*

    • Got a password; encode it for the server... / @@ -222,15 +231,6 @@ } # endif / APPLE */
  • if (http->status == HTTP_UNAUTHORIZED && http->digest_tries >= 3)

  • {
  • DEBUG_printf(("1cupsDoAuthentication: too many Negotiate tries (%d)",
  • http->digest_tries));

- http->status = HTTP_AUTHORIZATION_CANCELED;

  • return (-1);

- }

 if (http->gssname == GSS_C_NO_NAME)
 {
   if ((gss_service_name = getenv("CUPS_GSSSERVICENAME")) == NULL)
@michaelrsweet michaelrsweet added this to the Stable milestone Mar 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment