From 67b33605cab4817bf82471de436e46381f851c87 Mon Sep 17 00:00:00 2001 From: Jeff Trawick Date: Thu, 15 Mar 2001 22:09:27 +0000 Subject: [PATCH] Fix a security exposure in mod_access. Previously when IPv6 listening sockets were used, allow/deny-from-IPv4-address rules were not evaluated properly (PR #7407). Also, add the ability to specify IPv6 address strings with optional prefix length on Allow and Deny. Note: A bit more of PR #7407 remains dealing with a problem with double-reverse lookups when an IPv6 listening socket is used. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@88522 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 6 ++ modules/aaa/mod_access.c | 129 ++++++++------------------------------- 2 files changed, 33 insertions(+), 102 deletions(-) diff --git a/CHANGES b/CHANGES index 9b0fa4389d..0f3944086e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,5 +1,11 @@ Changes with Apache 2.0.15-dev + *) Fix a security exposure in mod_access. Previously when IPv6 + listening sockets were used, allow/deny-from-IPv4-address rules + were not evaluated properly (PR #7407). Also, add the ability to + specify IPv6 address strings with optional prefix length on Allow + and Deny. [Jeff Trawick] + *) Enhance rotatelogs so that a UTC offset can be specified, and the logfile name can be formatted using strftime(3). (Brought forward from 1.3.) [Ken Coar] diff --git a/modules/aaa/mod_access.c b/modules/aaa/mod_access.c index 39440ff5cc..ac9f275cbe 100644 --- a/modules/aaa/mod_access.c +++ b/modules/aaa/mod_access.c @@ -94,10 +94,7 @@ typedef struct { int limited; union { char *from; - struct { - unsigned long net; - unsigned long mask; - } ip; + apr_ipsubnet_t *ip; } x; enum allowdeny_type type; } allowdeny; @@ -150,20 +147,15 @@ static const char *order(cmd_parms *cmd, void *dv, const char *arg) return NULL; } -static int is_ip(const char *host) -{ - while ((*host == '.') || apr_isdigit(*host)) - host++; - return (*host == '\0'); -} - static const char *allow_cmd(cmd_parms *cmd, void *dv, const char *from, const char *where_c) { access_dir_conf *d = (access_dir_conf *) dv; allowdeny *a; - char *s; char *where = apr_pstrdup(cmd->pool, where_c); + char *s; + char msgbuf[120]; + apr_status_t rv; if (strcasecmp(from, "from")) return "allow and deny must be followed by 'from'"; @@ -179,92 +171,29 @@ static const char *allow_cmd(cmd_parms *cmd, void *dv, const char *from, } else if (!strcasecmp(where, "all")) { a->type = T_ALL; - } else if ((s = strchr(where, '/'))) { - unsigned long mask; - - a->type = T_IP; - /* trample on where, we won't be using it any more */ - *s++ = '\0'; - - if (!is_ip(where) - || (a->x.ip.net = apr_inet_addr(where)) == APR_INADDR_NONE) { - a->type = T_FAIL; - return "syntax error in network portion of network/netmask"; - } - - /* is_ip just tests if it matches [\d.]+ */ - if (!is_ip(s)) { - a->type = T_FAIL; - return "syntax error in mask portion of network/netmask"; - } - /* is it in /a.b.c.d form? */ - if (strchr(s, '.')) { - mask = apr_inet_addr(s); - if (mask == APR_INADDR_NONE) { - a->type = T_FAIL; - return "syntax error in mask portion of network/netmask"; - } - } - else { - /* assume it's in /nnn form */ - mask = atoi(s); - if (mask > 32 || mask <= 0) { - a->type = T_FAIL; - return "invalid mask in network/netmask"; - } - mask = 0xFFFFFFFFUL << (32 - mask); - mask = htonl(mask); - } - a->x.ip.mask = mask; - a->x.ip.net = (a->x.ip.net & mask); /* pjr - This fixes PR 4770 */ + *s++ = '\0'; + rv = apr_ipsubnet_create(&a->x.ip, where, s, cmd->pool); + switch(rv) { + case APR_SUCCESS: + break; + case APR_EINVAL: /* looked nothing like an IP address */ + return "An IP address was expected"; + default: + apr_strerror(rv, msgbuf, sizeof msgbuf); + return apr_pstrdup(cmd->pool, msgbuf); + } + a->type = T_IP; } - else if (apr_isdigit(*where) && is_ip(where)) { - /* legacy syntax for ip addrs: a.b.c. ==> a.b.c.0/24 for example */ - int shift; - char *t; - int octet; - - a->type = T_IP; - /* parse components */ - s = where; - a->x.ip.net = 0; - a->x.ip.mask = 0; - shift = 24; - while (*s) { - t = s; - if (!apr_isdigit(*t)) { - a->type = T_FAIL; - return "invalid ip address"; - } - while (apr_isdigit(*t)) { - ++t; - } - if (*t == '.') { - *t++ = 0; - } - else if (*t) { - a->type = T_FAIL; - return "invalid ip address"; - } - if (shift < 0) { - return "invalid ip address, only 4 octets allowed"; - } - octet = atoi(s); - if (octet < 0 || octet > 255) { - a->type = T_FAIL; - return "each octet must be between 0 and 255 inclusive"; - } - a->x.ip.net |= octet << shift; - a->x.ip.mask |= 0xFFUL << shift; - s = t; - shift -= 8; - } - a->x.ip.net = ntohl(a->x.ip.net); - a->x.ip.mask = ntohl(a->x.ip.mask); + else if ((rv = apr_ipsubnet_create(&a->x.ip, where, NULL, cmd->pool)) != APR_EINVAL) { + if (rv != APR_SUCCESS) { + apr_strerror(rv, msgbuf, sizeof msgbuf); + return apr_pstrdup(cmd->pool, msgbuf); + } + a->type = T_IP; } - else { + else { /* no slash, didn't look like an IP address => must be a host */ a->type = T_HOST; } @@ -330,14 +259,10 @@ static int find_allowdeny(request_rec *r, apr_array_header_t *a, int method) return 1; case T_IP: - /* XXX handle IPv6 with separate T_IP6 type or add common - * address masking operations to APR */ - if (ap[i].x.ip.net != APR_INADDR_NONE - && (r->connection->remote_addr->sa.sin.sin_addr.s_addr - & ap[i].x.ip.mask) == ap[i].x.ip.net) { - return 1; - } - break; + if (apr_ipsubnet_test(ap[i].x.ip, r->connection->remote_addr)) { + return 1; + } + break; case T_HOST: if (!gothost) { -- 2.40.0