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);
