httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Eland <and...@andreweland.org>
Subject [PATCH] Make "Symbolic link not allowed" log message more verbose
Date Mon, 17 Feb 2003 10:04:58 GMT

Hi,

After spending some time wondering why a particular symbolic link wasn't
allowed, I made a patch (against 2.0.44, server/request.c) that adds one
of the following reasons to the "Symbolic link not allowed" message:
* FollowSymLinks option not enabled
* owner doesn't match
* couldn't stat file

I hope somebody thinks it's useful.

  -- Andrew Eland (http://www.andreweland.org)

--- request-old.c	Sat Feb  8 16:55:49 2003
+++ request.c	Sat Feb  8 18:17:54 2003
@@ -378,24 +378,34 @@
  * we start off with an lstat().  Every lstat() must be dereferenced in case
  * it points at a 'nasty' - we must always rerun check_safe_file (or similar.)
  */
-static int resolve_symlink(char *d, apr_finfo_t *lfi, int opts, apr_pool_t *p)
+typedef enum {
+    SYMLINK_OK,
+    SYMLINK_NOT_ENABLED,
+    SYMLINK_STAT_FAILED,
+    SYMLINK_OWNER_DOES_NOT_MATCH,
+} resolve_symlink_result;
+
+static resolve_symlink_result resolve_symlink(char *d,
+                                              apr_finfo_t *lfi,
+                                              int opts,
+                                              apr_pool_t *p)
 {
     apr_finfo_t fi;
     int res;
     const char *savename;

     if (!(opts & (OPT_SYM_OWNER | OPT_SYM_LINKS))) {
-        return HTTP_FORBIDDEN;
+        return SYMLINK_NOT_ENABLED;
     }

     /* Save the name from the valid bits. */
     savename = (lfi->valid & APR_FINFO_NAME) ? lfi->name : NULL;

     if (opts & OPT_SYM_LINKS) {
-        if ((res = apr_stat(&fi, d, lfi->valid & ~(APR_FINFO_NAME
-                                                 | APR_FINFO_LINK), p))
-                 != APR_SUCCESS) {
-            return HTTP_FORBIDDEN;
+        if ((res = apr_stat(&fi, d, lfi->valid & ~(APR_FINFO_NAME
+                                                   | APR_FINFO_LINK), p))
+            != APR_SUCCESS) {
+            return SYMLINK_STAT_FAILED;
         }

         /* Give back the target */
@@ -405,7 +415,7 @@
             lfi->valid |= APR_FINFO_NAME;
         }

-        return OK;
+        return SYMLINK_OK;
     }

     /* OPT_SYM_OWNER only works if we can get the owner of
@@ -415,17 +425,17 @@
     if (!(lfi->valid & APR_FINFO_OWNER)) {
         if ((res = apr_lstat(&fi, d, lfi->valid | APR_FINFO_OWNER, p))
             != APR_SUCCESS) {
-            return HTTP_FORBIDDEN;
+        return SYMLINK_STAT_FAILED;
         }
     }

     if ((res = apr_stat(&fi, d, lfi->valid & ~(APR_FINFO_NAME), p))
         != APR_SUCCESS) {
-        return HTTP_FORBIDDEN;
+        return SYMLINK_STAT_FAILED;
     }

     if (apr_compare_users(fi.user, lfi->user) != APR_SUCCESS) {
-        return HTTP_FORBIDDEN;
+        return SYMLINK_OWNER_DOES_NOT_MATCH;
     }

     /* Give back the target */
@@ -435,9 +445,28 @@
         lfi->valid |= APR_FINFO_NAME;
     }

-    return OK;
+    return SYMLINK_OK;
 }

+static void log_resolve_symlink_result(resolve_symlink_result result,
+                                       const request_rec *r)
+{
+    const char *reason="";
+    switch(result) {
+    case SYMLINK_NOT_ENABLED:
+        reason=" (FollowSymLinks option not enabled)";
+        break;
+    case SYMLINK_STAT_FAILED:
+        reason=" (couldn't stat file)";
+        break;
+    case SYMLINK_OWNER_DOES_NOT_MATCH:
+        reason=" (owner doesn't match)";
+        break;
+    }
+    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                  "Symbolic link not allowed: %s%s",
+                  r->filename, reason);
+}

 /*
  * As we walk the directory configuration, the merged config won't
@@ -1017,12 +1046,11 @@
             if (thisinfo.filetype == APR_LNK) {
                 /* Is this a possibly acceptable symlink?
                  */
+                resolve_symlink_result res;
                 if ((res = resolve_symlink(r->filename, &thisinfo,
                                            opts.opts, r->pool)) != OK) {
-                    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                                  "Symbolic link not allowed: %s",
-                                  r->filename);
-                    return r->status = res;
+                    log_resolve_symlink_result(res, r);
+                    return r->status = HTTP_FORBIDDEN;
                 }
             }

@@ -1731,10 +1759,11 @@
         /*
          * Resolve this symlink.  We should tie this back to dir_walk's cache
          */
+        resolve_symlink_result res;
         if ((res = resolve_symlink(rnew->filename, &rnew->finfo,
                                    ap_allow_options(rnew), rnew->pool))
             != OK) {
-            rnew->status = res;
+            rnew->status = (res == SYMLINK_OK ? OK : HTTP_FORBIDDEN);
             return rnew;
         }
     }


Mime
View raw message