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

diff --git a/networking/httpd.c b/networking/httpd.c
index e3c1a4e1171fb4c7e393dd4d9dc12d1a41363cf2..382893bfbdef944b9fd317acd149011f93ccfb17 100644 (file)
--- 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