Bug 58528 - AllowOverrideList None gets misparsed and .htaccess files are parsed
Summary: AllowOverrideList None gets misparsed and .htaccess files are parsed
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: Core (show other bugs)
Version: 2.4.16
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-23 15:45 UTC by Michael Schlenker
Modified: 2016-04-01 12:37 UTC (History)
1 user (show)



Attachments
Patch for set_override_list() to properly handle 'None' (559 bytes, patch)
2015-10-26 10:33 UTC, Michael Schlenker
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Schlenker 2015-10-23 15:45:41 UTC
I am trying to get HTTPD 2.4.16 to not access any .htaccess files in the filesystem, but it seems to fail.

Checking with procmon (think strace for windows..), i see the httpd.exe process is touching a lot of .htaccess files with win32 CreateFile() when loading the favicon.ico.

It touches: d:\.htaccess and then every dir all the way up to the location where favicon.ico is stored. A debugger confirms it is done core/ap_directory_walk().

It should not touch the .htaccess parsing, but this code does not trigger:
  
  /* No htaccess in an incomplete root path,
   * nor if it's disabled
   */
  if (seg < startseg || (!opts.override && opts.override_list == NULL)) {
     break;
  }

opts.override_list is non-NULL (pointing to some address in memory).

Looking at the code that sets override_list (in core.c / set_override_list()), this smells fishy:

    d->override_list = apr_table_make(cmd->pool, argc);

    for (i=0;i<argc;i++){
        if (!strcasecmp(argv[i], "None")) {
            if (argc != 1) {
                return "'None' not allowed with other directives in "
                       "AllowOverrideList";
            }
            return NULL;
        }
    ...

d->override_list gets initialized with an empty table, but is not cleared in the 'None' branch, when the return NULL is hit. So the above check on opts.override_list == NULL would fail (if this value ends up there).

Not sure if that is the issue, but the effect is that .htaccess files get touched, even though they should not be touched.

I have a rather trivial httpd.conf:

LoadModule authz_core_module modules/mod_authz_core.so
LoadModule alias_module modules/mod_alias.so
LoadModule authz_host_module modules/mod_authz_host.so
LoadModule log_config_module modules/mod_log_config.so

<VirtualHost *:8080>
    ServerName                  localhost
    Protocol                    http
    DocumentRoot                "d:/code/trunk_clean/cdb"
</VirtualHost>

Listen 8080
PidFile "d:/temp/httpd.pid"
     
<Directory "/">
AllowOverride None
AllowOverrideList None
</Directory>
<Directory "d:/code/trunk_clean/cdb">
AllowOverride None
AllowOverrideList None
</Directory>

# Setup logging
ErrorLog "d:/temp/apache_error.log"
LogLevel warn

# We create our runtime files in the tempdir
DefaultRuntimeDir "d:/temp"

Alias "/favicon.ico" "d:/code/trunk_clean/cdb/w3/images/favicon.ico"
Comment 1 Michael Schlenker 2015-10-26 10:33:53 UTC
Created attachment 33221 [details]
Patch for set_override_list() to properly handle 'None'
Comment 2 Michael Schlenker 2015-10-26 10:37:19 UTC
I added a patch for the issue.

Testing with a broken .htaccess file, in one of the directories it is easily verifiable if .htaccess files get loaded and parsed when 'AllowOverride None' and 'AllowOverrideList None' are set. This leads to a start failure if the .htaccess file contains junk.

With the attached patch, the behaviour is better. .htaccess files do not get touched when AllowOverrideList is set to None (or left on default settings).
Comment 3 Daniel Ruggeri 2016-03-30 13:53:03 UTC
Thanks, Michael;
   The patch has the right idea for sure, but we took a slightly different path to fix in trunk to make the intention of the check during request processing more clear rather than just a null check.

I've committed the fix in trunk in r1737114 and proposed the fix for backport in 2.4's STATUS file.

Thanks!
Comment 4 Yann Ylavic 2016-04-01 12:37:15 UTC
Bakcported to 2.4.20 in r1737366.