httpd: fix bugs in authentication (by Peter Korsgaard <jacmet ATuclibc.org>)

 we were accepting empty username; also we were always checking
 dummy user:passwd pair ":" if user gave us wrong one.

function                                             old     new   delta
check_user_passwd                                    338     319     -19

1 files changed, 52 insertions, 46 deletions

diff --git a/networking/httpd.c b/networking/httpd.c
index e3c1a4e11..382893bfb 100644
--- a/networking/httpd.c
+++ b/networking/httpd.c
@@ -1664,70 +1664,76 @@ static int checkPermIP(void)
  *
  * Returns 1 if user_and_passwd is OK.
  */
-static int check_user_passwd(const char *path, const char *request)
+static int check_user_passwd(const char *path, const char *user_and_passwd)
 {
         Htaccess *cur;
-        const char *p;
-        const char *p0;
-
         const char *prev = NULL;
 
-        /* This could stand some work */
         for (cur = g_auth; cur; cur = cur->next) {
-                size_t l;
+                const char *dir_prefix;
+                size_t len;
+
+                dir_prefix = cur->before_colon;
+
+                /* WHY? */
+                /* If already saw a match, don't accept other different matches */
+                if (prev && strcmp(prev, dir_prefix) != 0)
+                        continue;
 
-                p0 = cur->before_colon;
-                if (prev != NULL && strcmp(prev, p0) != 0)
-                        continue;       /* find next identical */
-                p = cur->after_colon;
                 if (DEBUG)
-                        fprintf(stderr, "checkPerm: '%s' ? '%s'\n", p0, request);
+                        fprintf(stderr, "checkPerm: '%s' ? '%s'\n", dir_prefix, user_and_passwd);
 
-                l = strlen(p0);
-                if (strncmp(p0, path, l) == 0
-                 && (l == 1 || path[l] == '/' || path[l] == '\0')
+                /* If it's not a prefix match, continue searching */
+                len = strlen(dir_prefix);
+                if (len != 1 /* dir_prefix "/" matches all, don't need to check */
+                 && (strncmp(dir_prefix, path, len) != 0
+                    || (path[len] != '/' && path[len] != '\0'))
                 ) {
-                        char *u;
-                        /* path match found.  Check request */
-                        /* for check next /path:user:password */
-                        prev = p0;
-                        u = strchr(request, ':');
-                        if (u == NULL) {
-                                /* bad request, ':' required */
-                                break;
-                        }
+                        continue;
+                }
 
-                        if (ENABLE_FEATURE_HTTPD_AUTH_MD5) {
-                                char *pp;
+                /* Path match found */
+                prev = dir_prefix;
 
-                                if (strncmp(p, request, u - request) != 0) {
-                                        /* user doesn't match */
-                                        continue;
-                                }
-                                pp = strchr(p, ':');
-                                if (pp && pp[1] == '$' && pp[2] == '1'
-                                 && pp[3] == '$' && pp[4]
-                                ) {
-                                        char *encrypted = pw_encrypt(u+1, ++pp, 1);
-                                        int r = strcmp(encrypted, pp);
-                                        free(encrypted);
-                                        if (r == 0)
-                                                goto set_remoteuser_var;   /* Ok */
-                                        /* unauthorized */
+                if (ENABLE_FEATURE_HTTPD_AUTH_MD5) {
+                        char *md5_passwd;
+
+                        md5_passwd = strchr(cur->after_colon, ':');
+                        if (md5_passwd && md5_passwd[1] == '$' && md5_passwd[2] == '1'
+                         && md5_passwd[3] == '$' && md5_passwd[4]
+                        ) {
+                                char *encrypted;
+                                int r, user_len_p1;
+
+                                md5_passwd++;
+                                user_len_p1 = md5_passwd - cur->after_colon;
+                                /* comparing "user:" */
+                                if (strncmp(cur->after_colon, user_and_passwd, user_len_p1) != 0) {
                                         continue;
                                 }
+
+                                encrypted = pw_encrypt(
+                                        user_and_passwd + user_len_p1 /* cleartext pwd from user */,
+                                        md5_passwd /*salt */, 1 /* cleanup */);
+                                r = strcmp(encrypted, md5_passwd);
+                                free(encrypted);
+                                if (r == 0)
+                                        goto set_remoteuser_var; /* Ok */
+                                continue;
                         }
+                }
 
-                        if (strcmp(p, request) == 0) {
+                /* Comparing plaintext "user:pass" in one go */
+                if (strcmp(cur->after_colon, user_and_passwd) == 0) {
  set_remoteuser_var:
-                                remoteuser = xstrndup(request, u - request);
-                                return 1;   /* Ok */
-                        }
-                        /* unauthorized */
+                        remoteuser = xstrndup(user_and_passwd,
+                                        strchrnul(user_and_passwd, ':') - user_and_passwd);
+                        return 1; /* Ok */
                 }
         } /* for */
 
-        return prev == NULL;
+        /* 0(bad) if prev is set: matches were found but passwd was wrong */
+        return (prev == NULL);
 }
 #endif  /* FEATURE_HTTPD_BASIC_AUTH */
 
@@ -2039,7 +2045,7 @@ static void handle_incoming_and_exit(const len_and_sockaddr *fromAddr)
 #if ENABLE_FEATURE_HTTPD_BASIC_AUTH
         /* Case: no "Authorization:" was seen, but page does require passwd.
          * Check that with dummy user:pass */
-        if ((authorized <= 0) && check_user_passwd(urlcopy, ":") == 0) {
+        if ((authorized < 0) && check_user_passwd(urlcopy, ":") == 0) {
                 send_headers_and_exit(HTTP_UNAUTHORIZED);
         }
 #endif