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; } }
