The underlying rad_get_vendor_attr() function assumed that it would always be given valid VSA data. Indeed, the buffer length wasn't even passed in; the assumption was that the length field within the VSA structure would be valid. This could result in denial of service by providing a length that would be beyond the memory limit, or potential arbitrary memory access by providing a length greater than the actual data given. rad_get_vendor_attr() has been changed to require the raw data length be provided, and this is then used to check that the VSA is valid. /* {{{ proto string radius_get_vendor_attr(data) */ PHP_FUNCTION(radius_get_vendor_attr) { - int res; - const void *data; + const void *data, *raw; int len; u_int32_t vendor; + unsigned char type; + size_t data_len; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &data, &len) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &raw, &len) == FAILURE) { return; } - res = rad_get_vendor_attr(&vendor, &data, (size_t *) &len); - if (res == -1) { + if (rad_get_vendor_attr(&vendor, &type, &data, &data_len, raw, len) == -1) { RETURN_FALSE; } else { array_init(return_value); - add_assoc_long(return_value, "attr", res); + add_assoc_long(return_value, "attr", type); add_assoc_long(return_value, "vendor", vendor); - add_assoc_stringl(return_value, "data", (char *) data, len, 1); + add_assoc_stringl(return_value, "data", (char *) data, data_len, 1); return; } } @@ -898,15 +898,24 @@ struct rad_handle * } int -rad_get_vendor_attr(u_int32_t *vendor, const void **data, size_t *len) +rad_get_vendor_attr(u_int32_t *vendor, unsigned char *type, const void **data, size_t *len, const void *raw, size_t raw_len) { struct vendor_attribute *attr; - attr = (struct vendor_attribute *)*data; + if (raw_len < sizeof(struct vendor_attribute)) { + return -1; + } + + attr = (struct vendor_attribute *) raw; *vendor = ntohl(attr->vendor_value); + *type = attr->attrib_type; *data = attr->attrib_data; *len = attr->attrib_len - 2; + if ((attr->attrib_len + 4) > raw_len) { + return -1; + } + return (attr->attrib_type); } radlib_vs.h @@ -74,7 +74,7 @@ struct rad_handle; -int rad_get_vendor_attr(u_int32_t *, const void **, size_t *); +int rad_get_vendor_attr(u_int32_t *, unsigned char *, const void **, size_t *, const void *, size_t); int rad_put_vendor_addr(struct rad_handle *, int, int, struct in_addr); int rad_put_vendor_attr(struct rad_handle *, int, int, const void *, size_t);