blob: 001d9e906e0cc1503fbe764e3750b78bd2efddbc [file] [log] [blame]
From 01a44c96dbd04936e9cb2501745a834a0b09d504 Mon Sep 17 00:00:00 2001
From: Amos Jeffries <yadij@users.noreply.github.com>
Date: Sun, 13 May 2018 06:57:41 +0000
Subject: [PATCH] Bug 4843 pt1: ext_edirectory_userip_acl refactoring for GCC-8
(#204)
Proposed changes to this helper to fix strcat / strncat buffer
overread / overflow issues.
The approach takes three parts:
* adds a makeHexString function to replace many for-loops
catenating bits of strings together with hex conversion into a
second buffer. Replacing with a snprintf() and buffer overflow
handling.
* a copy of Ip::Address::lookupHostIp to convert the input
string into IP address binary format, then generate the hex
string using the above new hex function instead of looped
sub-string concatenations across several buffers.
This removes all the "00" and "0000" strncat() calls and
allows far simpler code even with added buffer overflow
handling.
* replace multiple string part concatenations with a few simpler
calls to snprintf() for all the search_ip buffer constructions.
Adding buffer overflow handling as needed for the new calls.
---
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Upstream-Status: Backport
.../ext_edirectory_userip_acl.cc | 376 ++++++------------
1 file changed, 120 insertions(+), 256 deletions(-)
diff --git a/helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc b/helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc
index 63609e4..ad16bfd 100644
--- a/helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc
+++ b/helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc
@@ -67,6 +67,9 @@
#ifdef HAVE_LDAP_H
#include <ldap.h>
#endif
+#ifdef HAVE_NETDB_H
+#include <netdb.h>
+#endif
#ifdef HELPER_INPUT_BUFFER
#define EDUI_MAXLEN HELPER_INPUT_BUFFER
@@ -714,11 +717,14 @@ BindLDAP(edui_ldap_t *l, char *dn, char *pw, unsigned int t)
/* Copy details - dn and pw CAN be NULL for anonymous and/or TLS */
if (dn != NULL) {
+ if (strlen(dn) >= sizeof(l->dn))
+ return LDAP_ERR_OOB; /* DN too large */
+
if ((l->basedn[0] != '\0') && (strstr(dn, l->basedn) == NULL)) {
/* We got a basedn, but it's not part of dn */
- xstrncpy(l->dn, dn, sizeof(l->dn));
- strncat(l->dn, ",", 1);
- strncat(l->dn, l->basedn, strlen(l->basedn));
+ const int x = snprintf(l->dn, sizeof(l->dn)-1, "%s,%s", dn, l->basedn);
+ if (x < 0 || static_cast<size_t>(x) >= sizeof(l->dn))
+ return LDAP_ERR_OOB; /* DN too large */
} else
xstrncpy(l->dn, dn, sizeof(l->dn));
}
@@ -778,24 +784,73 @@ BindLDAP(edui_ldap_t *l, char *dn, char *pw, unsigned int t)
}
}
+// XXX: duplicate (partial) of Ip::Address::lookupHostIp
+/**
+ * Convert the IP address string representation in src to
+ * its binary representation.
+ *
+ * \return binary representation of the src IP address.
+ * Must be free'd using freeaddrinfo().
+ */
+static struct addrinfo *
+makeIpBinary(const char *src)
+{
+ struct addrinfo want;
+ memset(&want, 0, sizeof(want));
+ want.ai_flags = AI_NUMERICHOST; // prevent actual DNS lookups!
+
+ struct addrinfo *dst = nullptr;
+ if (getaddrinfo(src, nullptr, &want, &dst) != 0) {
+ // not an IP address
+ /* free any memory getaddrinfo() dynamically allocated. */
+ if (dst)
+ freeaddrinfo(dst);
+ return nullptr;
+ }
+
+ return dst;
+}
+
+/**
+ * Convert srcLen bytes from src into HEX and store into dst, which
+ * has a maximum content size of dstSize including c-string terminator.
+ * The dst value produced will be a 0-terminated c-string.
+ *
+ * \retval N length of dst written (excluding c-string terminator)
+ * \retval -11 (LDAP_ERR_OOB) buffer overflow detected
+ */
+static int
+makeHexString(char *dst, const int dstSize, const char *src, const int srcLen)
+{
+ // HEX encoding doubles the amount of bytes/octets copied
+ if ((srcLen*2) >= dstSize)
+ return LDAP_ERR_OOB; // cannot copy that many
+
+ *dst = 0;
+
+ for (int k = 0; k < srcLen; ++k) {
+ int c = static_cast<int>(src[k]);
+ if (c < 0)
+ c = c + 256;
+ char hexc[4];
+ const int hlen = snprintf(hexc, sizeof(hexc), "%02X", c);
+ if (hlen < 0 || static_cast<size_t>(hlen) > sizeof(hexc)) // should be impossible
+ return LDAP_ERR_OOB;
+ strcat(dst, hexc);
+ }
+ return strlen(dst);
+}
+
/*
* ConvertIP() - <edui_ldap_t> <ip>
*
* Take an IPv4 address in dot-decimal or IPv6 notation, and convert to 2-digit HEX stored in l->search_ip
* This is the networkAddress that we search LDAP for.
- *
- * PENDING -- CHANGE OVER TO inet*_pton, but inet6_pton does not provide the correct syntax
- *
*/
static int
ConvertIP(edui_ldap_t *l, char *ip)
{
- char bufa[EDUI_MAXLEN], bufb[EDUI_MAXLEN], obj[EDUI_MAXLEN];
- char hexc[4], *p;
void *y, *z;
- size_t s;
- long x;
- int i, j, t, swi; /* IPv6 "::" cut over toggle */
if (l == NULL) return LDAP_ERR_NULL;
if (ip == NULL) return LDAP_ERR_PARAM;
if (!(l->status & LDAP_INIT_S)) return LDAP_ERR_INIT; /* Not initalized */
@@ -831,183 +886,22 @@ ConvertIP(edui_ldap_t *l, char *ip)
l->status |= (LDAP_IPV4_S);
z = NULL;
}
- s = strlen(ip);
- *(bufa) = '\0';
- *(bufb) = '\0';
- *(obj) = '\0';
- /* StringSplit() will zero out bufa & obj at each call */
- memset(l->search_ip, '\0', sizeof(l->search_ip));
- xstrncpy(bufa, ip, sizeof(bufa)); /* To avoid segfaults, use bufa instead of ip */
- swi = 0;
- if (l->status & LDAP_IPV6_S) {
- /* Search for :: in string */
- if ((bufa[0] == ':') && (bufa[1] == ':')) {
- /* bufa starts with a ::, so just copy and clear */
- xstrncpy(bufb, bufa, sizeof(bufb));
- *(bufa) = '\0';
- ++swi; /* Indicates that there is a bufb */
- } else if ((bufa[0] == ':') && (bufa[1] != ':')) {
- /* bufa starts with a :, a typo so just fill in a ':', cat and clear */
- bufb[0] = ':';
- strncat(bufb, bufa, strlen(bufa));
- *(bufa) = '\0';
- ++swi; /* Indicates that there is a bufb */
- } else {
- p = strstr(bufa, "::");
- if (p != NULL) {
- /* Found it, break bufa down and split into bufb here */
- *(bufb) = '\0';
- i = strlen(p);
- memcpy(bufb, p, i);
- *p = '\0';
- bufb[i] = '\0';
- ++swi; /* Indicates that there is a bufb */
- }
- }
- }
- s = strlen(bufa);
- if (s < 1)
- s = strlen(bufb);
- while (s > 0) {
- if ((l->status & LDAP_IPV4_S) && (swi == 0)) {
- /* Break down IPv4 address */
- t = StringSplit(bufa, '.', obj, sizeof(obj));
- if (t > 0) {
- errno = 0;
- x = strtol(obj, (char **)NULL, 10);
- if (((x < 0) || (x > 255)) || ((errno != 0) && (x == 0)) || ((obj[0] != '0') && (x == 0)))
- return LDAP_ERR_OOB; /* Out of bounds -- Invalid address */
- memset(hexc, '\0', sizeof(hexc));
- int hlen = snprintf(hexc, sizeof(hexc), "%02X", (int)x);
- strncat(l->search_ip, hexc, hlen);
- } else
- break; /* reached end of octet */
- } else if (l->status & LDAP_IPV6_S) {
- /* Break down IPv6 address */
- if (swi > 1)
- t = StringSplit(bufb, ':', obj, sizeof(obj)); /* After "::" */
- else
- t = StringSplit(bufa, ':', obj, sizeof(obj)); /* Before "::" */
- /* Convert octet by size (t) - and fill 0's */
- switch (t) { /* IPv6 is already in HEX, copy contents */
- case 4:
- hexc[0] = (char) toupper((int)obj[0]);
- i = (int)hexc[0];
- if (!isxdigit(i))
- return LDAP_ERR_OOB; /* Out of bounds */
- hexc[1] = (char) toupper((int)obj[1]);
- i = (int)hexc[1];
- if (!isxdigit(i))
- return LDAP_ERR_OOB; /* Out of bounds */
- hexc[2] = '\0';
- strncat(l->search_ip, hexc, 2);
- hexc[0] = (char) toupper((int)obj[2]);
- i = (int)hexc[0];
- if (!isxdigit(i))
- return LDAP_ERR_OOB; /* Out of bounds */
- hexc[1] = (char) toupper((int)obj[3]);
- i = (int)hexc[1];
- if (!isxdigit(i))
- return LDAP_ERR_OOB; /* Out of bounds */
- hexc[2] = '\0';
- strncat(l->search_ip, hexc, 2);
- break;
- case 3:
- hexc[0] = '0';
- hexc[1] = (char) toupper((int)obj[0]);
- i = (int)hexc[1];
- if (!isxdigit(i))
- return LDAP_ERR_OOB; /* Out of bounds */
- hexc[2] = '\0';
- strncat(l->search_ip, hexc, 2);
- hexc[0] = (char) toupper((int)obj[1]);
- i = (int)hexc[0];
- if (!isxdigit(i))
- return LDAP_ERR_OOB; /* Out of bounds */
- hexc[1] = (char) toupper((int)obj[2]);
- i = (int)hexc[1];
- if (!isxdigit(i))
- return LDAP_ERR_OOB; /* Out of bounds */
- hexc[2] = '\0';
- strncat(l->search_ip, hexc, 2);
- break;
- case 2:
- strncat(l->search_ip, "00", 2);
- hexc[0] = (char) toupper((int)obj[0]);
- i = (int)hexc[0];
- if (!isxdigit(i))
- return LDAP_ERR_OOB; /* Out of bounds */
- hexc[1] = (char) toupper((int)obj[1]);
- i = (int)hexc[1];
- if (!isxdigit(i))
- return LDAP_ERR_OOB; /* Out of bounds */
- hexc[2] = '\0';
- strncat(l->search_ip, hexc, 2);
- break;
- case 1:
- strncat(l->search_ip, "00", 2);
- hexc[0] = '0';
- hexc[1] = (char) toupper((int)obj[0]);
- i = (int)hexc[1];
- if (!isxdigit(i))
- return LDAP_ERR_OOB; /* Out of bounds */
- hexc[2] = '\0';
- strncat(l->search_ip, hexc, 2);
- break;
- default:
- if (t > 4)
- return LDAP_ERR_OOB;
- break;
- }
- /* Code to pad the address with 0's between a '::' */
- if ((strlen(bufa) == 0) && (swi == 1)) {
- /* We are *AT* the split, pad in some 0000 */
- t = strlen(bufb);
- /* How many ':' exist in bufb ? */
- j = 0;
- for (i = 0; i < t; ++i) {
- if (bufb[i] == ':')
- ++j;
- }
- --j; /* Preceding "::" doesn't count */
- t = 8 - (strlen(l->search_ip) / 4) - j; /* Remainder */
- if (t > 0) {
- for (i = 0; i < t; ++i)
- strncat(l->search_ip, "0000", 4);
- }
- }
- }
- if ((bufa[0] == '\0') && (swi > 0)) {
- s = strlen(bufb);
- ++swi;
- } else
- s = strlen(bufa);
- }
- s = strlen(l->search_ip);
- /* CHECK sizes of address, truncate or pad */
- /* if "::" is at end of ip, then pad another block or two */
- while ((l->status & LDAP_IPV6_S) && (s < 32)) {
- strncat(l->search_ip, "0000", 4);
- s = strlen(l->search_ip);
- }
- if ((l->status & LDAP_IPV6_S) && (s > 32)) {
- /* Too long, truncate */
- l->search_ip[32] = '\0';
- s = strlen(l->search_ip);
- }
- /* If at end of ip, and its not long enough, then pad another block or two */
- while ((l->status & LDAP_IPV4_S) && (s < 8)) {
- strncat(l->search_ip, "00", 2);
- s = strlen(l->search_ip);
- }
- if ((l->status & LDAP_IPV4_S) && (s > 8)) {
- /* Too long, truncate */
- l->search_ip[8] = '\0';
- s = strlen(l->search_ip);
+ size_t s = LDAP_ERR_INVALID;
+ if (struct addrinfo *dst = makeIpBinary(ip)) {
+ if (dst->ai_family == AF_INET6) {
+ struct sockaddr_in6 *sia = reinterpret_cast<struct sockaddr_in6 *>(dst->ai_addr);
+ const char *ia = reinterpret_cast<const char *>(sia->sin6_addr.s6_addr);
+ s = makeHexString(l->search_ip, sizeof(l->search_ip), ia, 16); // IPv6 = 16-byte address
+
+ } else if (dst->ai_family == AF_INET) {
+ struct sockaddr_in *sia = reinterpret_cast<struct sockaddr_in *>(dst->ai_addr);
+ const char *ia = reinterpret_cast<const char *>(&(sia->sin_addr));
+ s = makeHexString(l->search_ip, sizeof(l->search_ip), ia, 4); // IPv4 = 4-byte address
+ } // else leave s with LDAP_ERR_INVALID value
+ freeaddrinfo(dst);
}
- /* Completed, s is length of address in HEX */
return s;
}
@@ -1099,48 +993,42 @@ SearchFilterLDAP(edui_ldap_t *l, char *group)
}
if (group == NULL) {
/* No groupMembership= to add, yay! */
- xstrncpy(bufa, "(&", sizeof(bufa));
- strncat(bufa, edui_conf.search_filter, strlen(edui_conf.search_filter));
/* networkAddress */
- snprintf(bufb, sizeof(bufb), "(|(networkAddress=1\\23%s)", bufc);
if (l->status & LDAP_IPV4_S) {
- int ln = snprintf(bufd, sizeof(bufd), "(networkAddress=8\\23\\00\\00%s)(networkAddress=9\\23\\00\\00%s))", \
- bufc, bufc);
- strncat(bufb, bufd, ln);
+ const int ln = snprintf(bufd, sizeof(bufd), "(networkAddress=8\\23\\00\\00%s)(networkAddress=9\\23\\00\\00%s)", bufc, bufc);
+ if (ln < 0 || static_cast<size_t>(ln) >= sizeof(bufd))
+ return LDAP_ERR_OOB;
+
} else if (l->status & LDAP_IPV6_S) {
- int ln = snprintf(bufd, sizeof(bufd), "(networkAddress=10\\23\\00\\00%s)(networkAddress=11\\23\\00\\00%s))", \
- bufc, bufc);
- strncat(bufb, bufd, ln);
- } else
- strncat(bufb, ")", 1);
- strncat(bufa, bufb, strlen(bufb));
- strncat(bufa, ")", 1);
+ const int ln = snprintf(bufd, sizeof(bufd), "(networkAddress=10\\23\\00\\00%s)(networkAddress=11\\23\\00\\00%s)", bufc, bufc);
+ if (ln < 0 || static_cast<size_t>(ln) >= sizeof(bufd))
+ return LDAP_ERR_OOB;
+ }
+ const int x = snprintf(bufa, sizeof(bufa), "(&%s(|(networkAddress=1\\23%s)%s))", edui_conf.search_filter, bufc, bufd);
+ if (x < 0 || static_cast<size_t>(x) >= sizeof(bufa))
+ return LDAP_ERR_OOB;
+
} else {
/* Needs groupMembership= to add... */
- xstrncpy(bufa, "(&(&", sizeof(bufa));
- strncat(bufa, edui_conf.search_filter, strlen(edui_conf.search_filter));
/* groupMembership -- NOTE: Squid *MUST* provide "cn=" from squid.conf */
- snprintf(bufg, sizeof(bufg), "(groupMembership=%s", group);
if ((l->basedn[0] != '\0') && (strstr(group, l->basedn) == NULL)) {
- strncat(bufg, ",", 1);
- strncat(bufg, l->basedn, strlen(l->basedn));
+ const int ln = snprintf(bufg, sizeof(bufg), ",%s", l->basedn);
+ if (ln < 0 || static_cast<size_t>(ln) >= sizeof(bufd))
+ return LDAP_ERR_OOB;
}
- strncat(bufg, ")", 1);
- strncat(bufa, bufg, strlen(bufg));
/* networkAddress */
- snprintf(bufb, sizeof(bufb), "(|(networkAddress=1\\23%s)", bufc);
if (l->status & LDAP_IPV4_S) {
- int ln = snprintf(bufd, sizeof(bufd), "(networkAddress=8\\23\\00\\00%s)(networkAddress=9\\23\\00\\00%s))", \
- bufc, bufc);
- strncat(bufb, bufd, ln);
+ const int ln = snprintf(bufd, sizeof(bufd), "(networkAddress=8\\23\\00\\00%s)(networkAddress=9\\23\\00\\00%s)", bufc, bufc);
+ if (ln < 0 || static_cast<size_t>(ln) >= sizeof(bufd))
+ return LDAP_ERR_OOB;
} else if (l->status & LDAP_IPV6_S) {
- int ln = snprintf(bufd, sizeof(bufd), "(networkAddress=10\\23\\00\\00%s)(networkAddress=11\\23\\00\\00%s))", \
- bufc, bufc);
- strncat(bufb, bufd, ln);
- } else
- strncat(bufb, ")", 1);
- strncat(bufa, bufb, strlen(bufb));
- strncat(bufa, "))", 2);
+ const int ln = snprintf(bufd, sizeof(bufd), "(networkAddress=10\\23\\00\\00%s)(networkAddress=11\\23\\00\\00%s)", bufc, bufc);
+ if (ln < 0 || static_cast<size_t>(ln) >= sizeof(bufd))
+ return LDAP_ERR_OOB;
+ }
+ const int x = snprintf(bufa, sizeof(bufa), "(&(&%s(groupMembership=%s%s)(|(networkAddress=1\\23%s)%s)))", edui_conf.search_filter, group, bufg, bufc, bufd);
+ if (x < 0 || static_cast<size_t>(x) >= sizeof(bufa))
+ return LDAP_ERR_OOB;
}
s = strlen(bufa);
xstrncpy(l->search_filter, bufa, sizeof(l->search_filter));
@@ -1212,10 +1100,10 @@ static int
SearchIPLDAP(edui_ldap_t *l)
{
ber_len_t i, x;
- ber_len_t j, k;
- ber_len_t y, z;
- int c;
- char bufa[EDUI_MAXLEN], bufb[EDUI_MAXLEN], hexc[4];
+ ber_len_t j;
+ ber_len_t z;
+ char bufa[EDUI_MAXLEN];
+ char bufb[EDUI_MAXLEN];
LDAPMessage *ent;
if (l == NULL) return LDAP_ERR_NULL;
if (l->lp == NULL) return LDAP_ERR_POINTER;
@@ -1273,19 +1161,11 @@ SearchIPLDAP(edui_ldap_t *l)
/* bufa is the address, just compare it */
if (!(l->status & LDAP_IPV4_S) || (l->status & LDAP_IPV6_S))
break; /* Not looking for IPv4 */
- for (k = 0; k < z; ++k) {
- c = (int) bufa[k];
- if (c < 0)
- c = c + 256;
- int hlen = snprintf(hexc, sizeof(hexc), "%02X", c);
- if (k == 0)
- xstrncpy(bufb, hexc, sizeof(bufb));
- else
- strncat(bufb, hexc, hlen);
- }
- y = strlen(bufb);
+ const int blen = makeHexString(bufb, sizeof(bufb), bufa, z);
+ if (blen < 0)
+ return blen;
/* Compare value with IP */
- if (memcmp(l->search_ip, bufb, y) == 0) {
+ if (memcmp(l->search_ip, bufb, blen) == 0) {
/* We got a match! - Scan 'ber' for 'cn' values */
z = ldap_count_values_len(ber);
for (j = 0; j < z; ++j) {
@@ -1308,19 +1188,11 @@ SearchIPLDAP(edui_ldap_t *l)
/* bufa + 2 is the address (skip 2 digit port) */
if (!(l->status & LDAP_IPV4_S) || (l->status & LDAP_IPV6_S))
break; /* Not looking for IPv4 */
- for (k = 2; k < z; ++k) {
- c = (int) bufa[k];
- if (c < 0)
- c = c + 256;
- int hlen = snprintf(hexc, sizeof(hexc), "%02X", c);
- if (k == 2)
- xstrncpy(bufb, hexc, sizeof(bufb));
- else
- strncat(bufb, hexc, hlen);
- }
- y = strlen(bufb);
+ const int blen = makeHexString(bufb, sizeof(bufb), &bufa[2], z);
+ if (blen < 0)
+ return blen;
/* Compare value with IP */
- if (memcmp(l->search_ip, bufb, y) == 0) {
+ if (memcmp(l->search_ip, bufb, blen) == 0) {
/* We got a match! - Scan 'ber' for 'cn' values */
z = ldap_count_values_len(ber);
for (j = 0; j < z; ++j) {
@@ -1343,19 +1215,11 @@ SearchIPLDAP(edui_ldap_t *l)
/* bufa + 2 is the address (skip 2 digit port) */
if (!(l->status & LDAP_IPV6_S))
break; /* Not looking for IPv6 */
- for (k = 2; k < z; ++k) {
- c = (int) bufa[k];
- if (c < 0)
- c = c + 256;
- int hlen = snprintf(hexc, sizeof(hexc), "%02X", c);
- if (k == 2)
- xstrncpy(bufb, hexc, sizeof(bufb));
- else
- strncat(bufb, hexc, hlen);
- }
- y = strlen(bufb);
+ const int blen = makeHexString(bufb, sizeof(bufb), &bufa[2], z);
+ if (blen < 0)
+ return blen;
/* Compare value with IP */
- if (memcmp(l->search_ip, bufb, y) == 0) {
+ if (memcmp(l->search_ip, bufb, blen) == 0) {
/* We got a match! - Scan 'ber' for 'cn' values */
z = ldap_count_values_len(ber);
for (j = 0; j < z; ++j) {