From 45a9371484881d335b2142ea898532e248b76f23 Mon Sep 17 00:00:00 2001 From: Arran Cudbard-Bell Date: Tue, 18 Mar 2014 15:14:36 +0000 Subject: [PATCH] Modify functions in print.c to return > than outlen if they run out of buffer. This is required for correct operation of rlm_rest stream encoders --- src/include/libradius.h | 3 + src/lib/print.c | 340 ++++++++++++++++---------------- src/modules/rlm_perl/rlm_perl.c | 6 +- src/modules/rlm_rest/rest.c | 4 +- 4 files changed, 179 insertions(+), 174 deletions(-) diff --git a/src/include/libradius.h b/src/include/libradius.h index 0c814b3ad38a..f6b8dec580b8 100644 --- a/src/include/libradius.h +++ b/src/include/libradius.h @@ -427,6 +427,9 @@ int fr_check_lib_magic(uint64_t magic); int fr_utf8_char(uint8_t const *str); size_t fr_print_string(char const *in, size_t inlen, char *out, size_t outlen); + +#define is_truncated(_ret, _max) ((_ret) <= (_max)) +#define truncate_len(_ret, _max) (((_ret) <= (_max)) ? _ret : (_max) - 1) size_t vp_prints_value(char *out, size_t outlen, VALUE_PAIR const *vp, int8_t quote); char *vp_aprinttype(TALLOC_CTX *ctx, PW_TYPE type); char *vp_aprint(TALLOC_CTX *ctx, VALUE_PAIR const *vp); diff --git a/src/lib/print.c b/src/lib/print.c index a1fb57174a5b..58548a9b3e5e 100644 --- a/src/lib/print.c +++ b/src/lib/print.c @@ -128,35 +128,28 @@ int fr_utf8_char(uint8_t const *str) */ size_t fr_print_string(char const *in, size_t inlen, char *out, size_t outlen) { - char const *start = out; - uint8_t const *str = (uint8_t const *) in; + uint8_t const *p = (uint8_t const *) in; int sp = 0; int utf8 = 0; + size_t freespace = outlen; if (!in) { - if (outlen) { - *out = '\0'; - } + if (outlen) *out = '\0'; return 0; } - if (inlen == 0) { - inlen = strlen(in); - } + if (inlen == 0) inlen = strlen(in); - /* - * - */ - while ((inlen > 0) && (outlen > 4)) { + while ((inlen > 0) && (freespace > 4)) { /* * Hack: never print trailing zero. - * Some clients send strings with an off-by-one + * Some clients send pings with an off-by-one * length (confused with strings in C). */ - if ((inlen == 1) && (*str == 0)) break; + if ((inlen == 1) && (*p == '\0')) break; - switch (*str) { + switch (*p) { case '\\': sp = '\\'; break; @@ -173,38 +166,39 @@ size_t fr_print_string(char const *in, size_t inlen, char *out, size_t outlen) sp = '"'; break; default: - sp = 0; + sp = '\0'; break; } if (sp) { *out++ = '\\'; *out++ = sp; - outlen -= 2; - str++; + freespace -= 2; + p++; inlen--; continue; } - utf8 = fr_utf8_char(str); + utf8 = fr_utf8_char(p); if (!utf8) { - snprintf(out, outlen, "\\%03o", *str); - out += 4; - outlen -= 4; - str++; + snprintf(out, freespace, "\\%03o", *p); + out += 4; + freespace -= 4; + p++; inlen--; continue; } do { - *out++ = *str++; - outlen--; + *out++ = *p++; + freespace--; inlen--; } while (--utf8 > 0); } *out = '\0'; - return out - start; + if (inlen > 0) return outlen + 4; + return outlen - freespace; } @@ -215,14 +209,16 @@ size_t fr_print_string(char const *in, size_t inlen, char *out, size_t outlen) * @param[in] vp to print. * @param[in] quote Char to add before and after printed value, if 0 no char will be added, if < 0 raw string will be * added. - * @return length of data written to out or 0 on error. + * @return the length of data written to out, or a value >= outlen on truncation. */ size_t vp_prints_value(char *out, size_t outlen, VALUE_PAIR const *vp, int8_t quote) { + DICT_VALUE *v; + char buf[1024]; /* Interim buffer to use with poorly behaved printing functions */ + char const *a = NULL; time_t t; struct tm s_tm; - char *start = out; size_t len, freespace = outlen; *out = '\0'; @@ -234,7 +230,7 @@ size_t vp_prints_value(char *out, size_t outlen, VALUE_PAIR const *vp, int8_t qu /* need to copy the escaped value, but quoted */ if (quote > 0) { if (freespace < 3) { - return 0; + return vp->length + 2; } *out++ = (char) quote; @@ -245,7 +241,7 @@ size_t vp_prints_value(char *out, size_t outlen, VALUE_PAIR const *vp, int8_t qu if (len >= (freespace - 1)) { out[outlen - 2] = (char) quote; out[outlen - 1] = '\0'; - return outlen - 1; + return len + 2; } out += len; freespace -= len; @@ -254,107 +250,120 @@ size_t vp_prints_value(char *out, size_t outlen, VALUE_PAIR const *vp, int8_t qu freespace--; *out = '\0'; - return out - start; + return len + 2; } /* xlat.c - need to copy raw value verbatim */ - if (quote < 0) { - strlcpy(out, vp->vp_strvalue, freespace); - return strlen(out); + else if (quote < 0) { + return strlcpy(out, vp->vp_strvalue, outlen); } - return fr_print_string(vp->vp_strvalue, vp->length, out, freespace); + + return fr_print_string(vp->vp_strvalue, vp->length, out, outlen); case PW_TYPE_INTEGER: + if (vp->da->flags.has_tag) { + /* Attribute value has a tag, need to ignore it */ + if ((v = dict_valbyattr(vp->da->attr, vp->da->vendor, (vp->vp_integer & 0xffffff))) != NULL) { + a = v->name; + len = strlen(a); + } else { + /* should never be truncated */ + len = snprintf(buf, sizeof(buf), "%u", (vp->vp_integer & 0xffffff)); + a = buf; + } + } else { case PW_TYPE_BYTE: case PW_TYPE_SHORT: - { - DICT_VALUE *v = dict_valbyattr(vp->da->attr, vp->da->vendor, vp->vp_integer); - len = v ? strlcpy(out, v->name, freespace) : - (size_t) snprintf(out, freespace, "%u", vp->vp_integer); - if (len >= freespace) { - return outlen - 1; + /* Normal, non-tagged attribute */ + if ((v = dict_valbyattr(vp->da->attr, vp->da->vendor, vp->vp_integer)) != NULL) { + a = v->name; + len = strlen(a); + } else { + /* should never be truncated */ + len = snprintf(buf, sizeof(buf), "%u", vp->vp_integer); + a = buf; + } } - return len; - } + break; case PW_TYPE_INTEGER64: - len = snprintf(out, freespace, "%" PRIu64, vp->vp_integer64); - if (len >= freespace) { - return outlen - 1; - } - return len; + return snprintf(out, outlen, "%" PRIu64, vp->vp_integer64); case PW_TYPE_DATE: t = vp->vp_date; if (quote > 0) { - if (freespace < 3) { - return 0; - } - - *out++ = (char) quote; - freespace--; - - len = strftime(out, freespace, "%b %e %Y %H:%M:%S %Z", localtime_r(&t, &s_tm)); - /* always terminate the quoted string with another quote */ - if (len >= (freespace - 1)) { - out[outlen - 2] = (char) quote; - out[outlen - 1] = '\0'; - return outlen - 1; - } - - out += len; - freespace -= len; - - *out++ = (char) quote; - freespace--; - *out = '\0'; - - return out - start; - } - - len = strftime(out, freespace, "%b %e %Y %H:%M:%S %Z", localtime_r(&t, &s_tm)); - if (len >= freespace) { - return outlen - 1; + len = strftime(buf, sizeof(buf) - 1, "%%%b %e %Y %H:%M:%S %Z%%", localtime_r(&t, &s_tm)); + buf[0] = (char) quote; + buf[len - 1] = (char) quote; + buf[len] = '\0'; + } else { + len = strftime(buf, sizeof(buf), "%b %e %Y %H:%M:%S %Z", localtime_r(&t, &s_tm)); } - return len; + a = buf; + break; case PW_TYPE_SIGNED: /* Damned code for 1 WiMAX attribute */ - len = snprintf(out, freespace, "%d", vp->vp_signed); - if (len >= freespace) { - return outlen - 1; - } - return len; + len = snprintf(buf, sizeof(buf), "%d", vp->vp_signed); + a = buf; + break; case PW_TYPE_IPADDR: - inet_ntop(AF_INET, &(vp->vp_ipaddr), out, freespace); - return strlen(out); + a = inet_ntop(AF_INET, &(vp->vp_ipaddr), buf, sizeof(buf)); + len = strlen(buf); + break; case PW_TYPE_ABINARY: #ifdef WITH_ASCEND_BINARY - print_abinary(out, freespace, vp, quote); - return strlen(out); + print_abinary(buf, sizeof(buf), vp, quote); + a = buf; + len = strlen(buf); + break; #else /* FALL THROUGH */ #endif - case PW_TYPE_TLV: case PW_TYPE_OCTETS: - if (outlen <= (2 * (vp->length + 1))) { - return 0; + case PW_TYPE_TLV: + { + size_t max; + + /* Return the number of bytes we would of written */ + len = (vp->length * 2) + 2; + if (freespace <= 1) { + return len; + } + + *out++ = '0'; + freespace--; + + if (freespace <= 1) { + *out = '\0'; + return len; + } + *out++ = 'x'; + freespace--; + + if (freespace <= 2) { + *out = '\0'; + return len; } - strcpy(out, "0x"); - out += 2; - out += fr_bin2hex(out, vp->vp_octets, vp->length); + /* Get maximum number of bytes we can encode given freespace */ + max = ((freespace % 2) ? freespace - 1 : freespace - 2) / 2; + fr_bin2hex(out, vp->vp_octets, (vp->length > max) ? max : vp->length); - return out - start; + return len; + } + break; case PW_TYPE_IFID: - ifid_ntoa(out, freespace, vp->vp_ifid); - return strlen(out); + a = ifid_ntoa(buf, sizeof(buf), vp->vp_ifid); + len = strlen(buf); + break; case PW_TYPE_IPV6ADDR: - inet_ntop(AF_INET6, &vp->vp_ipv6addr, out, freespace); - return strlen(out); + a = inet_ntop(AF_INET6, &vp->vp_ipv6addr, buf, sizeof(buf)); + len = strlen(buf); + break; case PW_TYPE_IPV6PREFIX: { @@ -364,19 +373,17 @@ size_t vp_prints_value(char *out, size_t outlen, VALUE_PAIR const *vp, int8_t qu * Alignment issues. */ memcpy(&addr, &(vp->vp_ipv6prefix[2]), sizeof(addr)); - if (inet_ntop(AF_INET6, &addr, out, freespace)) { - len = strlen(out); - out += len; - freespace -= len; - len = snprintf(out, freespace, "/%u", (unsigned int) vp->vp_ipv6prefix[1]); - if (len >= freespace) { - return outlen - 1; - } - out += len; + a = inet_ntop(AF_INET6, &addr, buf, sizeof(buf)); + if (a) { + char *p = buf; + + len = strlen(buf); + p += len; + len += snprintf(p, sizeof(buf) - len, "/%u", (unsigned int) vp->vp_ipv6prefix[1]); } - return out - start; } + break; case PW_TYPE_IPV4PREFIX: { @@ -386,42 +393,38 @@ size_t vp_prints_value(char *out, size_t outlen, VALUE_PAIR const *vp, int8_t qu * Alignment issues. */ memcpy(&addr, &(vp->vp_ipv4prefix[2]), sizeof(addr)); - if (inet_ntop(AF_INET, &addr, out, freespace)) { - len = strlen(out); - out += len; - freespace -= len; - len = snprintf(out, freespace, "/%u", (unsigned int) vp->vp_ipv4prefix[1]); - if (len >= freespace) { - return outlen - 1; - } - out += len; + a = inet_ntop(AF_INET, &addr, buf, sizeof(buf)); + if (a) { + char *p = buf; + len = strlen(buf); + p += len; + len += snprintf(p, sizeof(buf) - len, "/%u", (unsigned int) (vp->vp_ipv4prefix[1] & 0x3f)); } - return out - start; } + break; case PW_TYPE_ETHERNET: - len = snprintf(out, freespace, "%02x:%02x:%02x:%02x:%02x:%02x", - vp->vp_ether[0], vp->vp_ether[1], - vp->vp_ether[2], vp->vp_ether[3], - vp->vp_ether[4], vp->vp_ether[5]); - if (len >= freespace) { - return outlen - 1; - } - return len; + return snprintf(out, outlen, "%02x:%02x:%02x:%02x:%02x:%02x", + vp->vp_ether[0], vp->vp_ether[1], + vp->vp_ether[2], vp->vp_ether[3], + vp->vp_ether[4], vp->vp_ether[5]); default: - len = strlcpy(out, "UNKNOWN-TYPE", freespace); - if (len >= freespace) { - return outlen - 1; - } - return len; + a = "UNKNOWN-TYPE"; + len = strlen(a); + break; } - return 0; -} + if (a) { + strlcpy(out, a, outlen); + } else { + len = 0; + } + return len; /* Return the number of bytes we would of written (for truncation detection) */ +} char *vp_aprinttype(TALLOC_CTX *ctx, PW_TYPE type) { @@ -474,10 +477,6 @@ char *vp_aprint(TALLOC_CTX *ctx, VALUE_PAIR const *vp) { char *p; - if (!fr_assert(vp)) { - return NULL; - } - switch (vp->da->type) { case PW_TYPE_STRING: /* @@ -521,7 +520,7 @@ char *vp_aprint(TALLOC_CTX *ctx, VALUE_PAIR const *vp) #ifdef WITH_ASCEND_BINARY p = talloc_array(ctx, char, 128); if (!p) return NULL; - print_abinary(p, 128, vp, '\0'); + print_abinary(p, 128, vp, 0); break; #else /* FALL THROUGH */ @@ -536,8 +535,8 @@ char *vp_aprint(TALLOC_CTX *ctx, VALUE_PAIR const *vp) case PW_TYPE_DATE: { - time_t t; - struct tm s_tm; + time_t t; + struct tm s_tm; t = vp->vp_date; @@ -607,7 +606,6 @@ char *vp_aprint(TALLOC_CTX *ctx, VALUE_PAIR const *vp) */ size_t vp_prints_value_json(char *out, size_t outlen, VALUE_PAIR const *vp) { - char *start = out; char const *q; size_t len, freespace = outlen; @@ -618,26 +616,26 @@ size_t vp_prints_value_json(char *out, size_t outlen, VALUE_PAIR const *vp) case PW_TYPE_SHORT: if (vp->da->flags.has_value) break; - len = snprintf(out, freespace, "%u", vp->vp_integer); - return len; + return snprintf(out, freespace, "%u", vp->vp_integer); case PW_TYPE_SIGNED: - len = snprintf(out, freespace, "%d", vp->vp_signed); - return len; + return snprintf(out, freespace, "%d", vp->vp_signed); default: break; } } - if (freespace < 2) return -1; + /* Indicate truncation */ + if (freespace < 2) return outlen + 1; *out++ = '"'; freespace--; switch (vp->da->type) { case PW_TYPE_STRING: for (q = vp->vp_strvalue; q < vp->vp_strvalue + vp->length; q++) { - if (freespace < 3) return -1; + /* Indicate truncation */ + if (freespace < 3) return outlen + 1; if (*q == '"') { *out++ = '\\'; @@ -685,7 +683,7 @@ size_t vp_prints_value_json(char *out, size_t outlen, VALUE_PAIR const *vp) break; default: len = snprintf(out, freespace, "u%04X", *q); - if (len >= freespace) return outlen; + if (is_truncated(len, freespace)) return (outlen - freespace) + len; out += len; freespace -= len; } @@ -695,17 +693,18 @@ size_t vp_prints_value_json(char *out, size_t outlen, VALUE_PAIR const *vp) default: len = vp_prints_value(out, freespace, vp, 0); - if (len >= freespace) return outlen; + if (is_truncated(len, freespace)) return (outlen - freespace) + len; out += len; freespace -= len; break; } - if (freespace < 2) return outlen; + /* Indicate truncation */ + if (freespace < 2) return outlen + 1; *out++ = '"'; *out = '\0'; // We don't increment out, because the nul byte should not be included in the length - return out - start; + return outlen - freespace; } /* @@ -757,7 +756,6 @@ extern int fr_attr_mask[]; static size_t vp_print_attr_oid(char *out, size_t outlen, unsigned int attr, int dv_type) { int nest; - char *start = out; size_t len, freespace = outlen; switch (dv_type) { @@ -770,24 +768,24 @@ static size_t vp_print_attr_oid(char *out, size_t outlen, unsigned int attr, int default: case 1: len = snprintf(out, freespace, "%u", attr & 0xff); - if (len >= freespace) return outlen; + if (is_truncated(len, freespace)) return len; out += len; freespace -= len; break; } - if ((attr >> 8) == 0) return out - start; + if ((attr >> 8) == 0) return (outlen - freespace); for (nest = 1; nest <= fr_attr_max_tlv; nest++) { if (((attr >> fr_attr_shift[nest]) & fr_attr_mask[nest]) == 0) break; len = snprintf(out, freespace, ".%u", (attr >> fr_attr_shift[nest]) & fr_attr_mask[nest]); - if (len >= freespace) return outlen; - out += freespace; + if (is_truncated(len, freespace)) return (outlen - freespace) + len; + out += len; freespace -= len; } - return out - start; + return (outlen - freespace); } /** Print the names of attributes which are not in the dictionaries @@ -807,19 +805,20 @@ static size_t vp_print_attr_oid(char *out, size_t outlen, unsigned int attr, int size_t vp_print_name(char *out, size_t outlen, unsigned int attr, unsigned int vendor) { int dv_type = 1; - char *start = out; size_t len, freespace = outlen; if (!out) return 0; len = snprintf(out, freespace, "Attr-"); - if (len >= freespace) return outlen; + if (is_truncated(len, freespace)) { + return len; + } out += len; freespace -= len; if (vendor > FR_MAX_VENDOR) { len = snprintf(out, freespace, "%u.", vendor / FR_MAX_VENDOR); - if (len >= freespace) return outlen; + if (is_truncated(len, freespace)) return (outlen - freespace) + len; out += len; freespace -= len; @@ -835,16 +834,16 @@ size_t vp_print_name(char *out, size_t outlen, unsigned int attr, unsigned int v } len = snprintf(out, freespace, "26.%u.", vendor); - if (len >= freespace) return outlen; + if (is_truncated(len, freespace)) return (outlen - freespace) + len; out += len; freespace -= len; } len = vp_print_attr_oid(out, freespace, attr, dv_type); - if (len >= freespace) return outlen; - out += len; + if (is_truncated(len, freespace)) return (outlen - freespace) + len; + freespace -= len; - return out - start; + return (outlen - freespace); } @@ -864,7 +863,6 @@ size_t vp_print_name(char *out, size_t outlen, unsigned int attr, unsigned int v size_t vp_prints(char *out, size_t outlen, VALUE_PAIR const *vp) { char const *token = NULL; - char *start = out; size_t len, freespace = outlen; if (!out) return 0; @@ -880,20 +878,22 @@ size_t vp_prints(char *out, size_t outlen, VALUE_PAIR const *vp) token = ""; } - if(vp->da->flags.has_tag) { + if (vp->da->flags.has_tag) { len = snprintf(out, freespace, "%s:%d %s ", vp->da->name, vp->tag, token); } else { len = snprintf(out, freespace, "%s %s ", vp->da->name, token); } - if (len >= freespace) return outlen; + + if (is_truncated(len, freespace)) return len; out += len; freespace -= len; + len = vp_prints_value(out, freespace, vp, '\''); - if (len >= freespace) return outlen; - out += len; + if (is_truncated(len, freespace)) return (outlen - freespace) + len; + freespace -= len; - return out - start; + return (outlen - freespace); } @@ -940,7 +940,7 @@ void vp_print(FILE *fp, VALUE_PAIR const *vp) void vp_printlist(FILE *fp, VALUE_PAIR const *vp) { vp_cursor_t cursor; - for (vp = _fr_cursor_init(&cursor, &vp); vp; vp = fr_cursor_next(&cursor)) { + for (vp = fr_cursor_init(&cursor, &vp); vp; vp = fr_cursor_next(&cursor)) { vp_print(fp, vp); } } diff --git a/src/modules/rlm_perl/rlm_perl.c b/src/modules/rlm_perl/rlm_perl.c index dee109a937d3..bf6be0847b4c 100644 --- a/src/modules/rlm_perl/rlm_perl.c +++ b/src/modules/rlm_perl/rlm_perl.c @@ -488,7 +488,7 @@ static void perl_store_vps(TALLOC_CTX *ctx, VALUE_PAIR *vps, HV *rad_hv) char const *name; char namebuf[256]; char buffer[1024]; - int len; + size_t len; hv_undef(rad_hv); @@ -533,7 +533,7 @@ static void perl_store_vps(TALLOC_CTX *ctx, VALUE_PAIR *vps, HV *rad_hv) vp; vp = fr_cursor_next(&cursor)) { len = vp_prints_value(buffer, sizeof(buffer), vp, 0); - av_push(av, newSVpv(buffer, len)); + av_push(av, newSVpv(buffer, truncate_len(len, sizeof(buffer)))); } (void)hv_store(rad_hv, name, strlen(name), newRV_noinc((SV *)av), 0); @@ -543,7 +543,7 @@ static void perl_store_vps(TALLOC_CTX *ctx, VALUE_PAIR *vps, HV *rad_hv) */ } else { len = vp_prints_value(buffer, sizeof(buffer), sublist, 0); - (void)hv_store(rad_hv, name, strlen(name), newSVpv(buffer, len), 0); + (void)hv_store(rad_hv, name, strlen(name), newSVpv(buffer, truncate_len(len, sizeof(buffer))), 0); } pairfree(&sublist); diff --git a/src/modules/rlm_rest/rest.c b/src/modules/rlm_rest/rest.c index 2de9857451c6..f3b4ae4a13bf 100644 --- a/src/modules/rlm_rest/rest.c +++ b/src/modules/rlm_rest/rest.c @@ -498,6 +498,8 @@ static size_t rest_encode_post(void *out, size_t size, size_t nmemb, void *userd * Write out single attribute string. */ len = vp_prints_value(p, freespace, vp, 0); + if (is_truncated(len, freespace)) goto no_space; + RDEBUG3("\tLength : %zd", len); if (len > 0) { escaped = curl_escape(p, len); @@ -677,7 +679,7 @@ static size_t rest_encode_json(void *out, size_t size, size_t nmemb, void *userd for (;;) { len = vp_prints_value_json(p, freespace, vp); - if (len >= freespace) goto no_space; + if (is_truncated(len, freespace)) goto no_space; /* * Show actual value length minus quotes