Skip to content

[defect]: Error cases return 0 (non‑negative) instead of negative error in fr_value_box_from_network() #5665

@MegaManSec

Description

@MegaManSec

What type of defect/bug is this?

Non compliance with a standards document

How can the issue be reproduced?

dst->vb_ip.scope_id = scope_id;
len--;
}
FR_DBUFF_OUT_RETURN(&dst->vb_ip.prefix, &work_dbuff);
FR_DBUFF_OUT_MEMCPY_RETURN((uint8_t *)&dst->vb_ip.addr.v6, &work_dbuff, len - 1);
break;
case FR_TYPE_COMBO_IP_ADDR:
if ((len >= network_min_size(FR_TYPE_IPV6_ADDR)) &&
(len <= network_max_size(FR_TYPE_IPV6_ADDR))) goto ipv6addr; /* scope is optional */
else if ((len >= network_min_size(FR_TYPE_IPV4_ADDR)) &&
(len <= network_max_size(FR_TYPE_IPV4_ADDR))) goto ipv4addr;
fr_strerror_const("Invalid combo ip address value");
return 0;
case FR_TYPE_COMBO_IP_PREFIX:
if ((len >= network_min_size(FR_TYPE_IPV6_PREFIX)) &&
(len <= network_max_size(FR_TYPE_IPV6_PREFIX))) goto ipv6prefix; /* scope is optional */
else if ((len >= network_min_size(FR_TYPE_IPV4_PREFIX)) &&
(len <= network_max_size(FR_TYPE_IPV4_PREFIX))) goto ipv4prefix;
fr_strerror_const("Invalid combo ip prefix value");
return 0;
case FR_TYPE_BOOL:
{
uint8_t val = 0;
FR_DBUFF_OUT_RETURN(&val, &work_dbuff);
dst->datum.boolean = (val != 0);
}
break;
case FR_TYPE_IFID:
case FR_TYPE_ETHERNET:
FR_DBUFF_OUT_MEMCPY_RETURN(fr_value_box_raw(dst, type), &work_dbuff, len);
break;
case FR_TYPE_UINT8:
FR_DBUFF_OUT_RETURN(&dst->vb_uint8, &work_dbuff);
break;
case FR_TYPE_UINT16:
FR_DBUFF_OUT_RETURN(&dst->vb_uint16, &work_dbuff);
break;
case FR_TYPE_UINT32:
FR_DBUFF_OUT_RETURN(&dst->vb_uint32, &work_dbuff);
break;
case FR_TYPE_UINT64:
FR_DBUFF_OUT_RETURN(&dst->vb_uint64, &work_dbuff);
break;
case FR_TYPE_INT8:
FR_DBUFF_OUT_RETURN(&dst->vb_int8, &work_dbuff);
break;
case FR_TYPE_INT16:
FR_DBUFF_OUT_RETURN(&dst->vb_int16, &work_dbuff);
break;
case FR_TYPE_INT32:
FR_DBUFF_OUT_RETURN(&dst->vb_int32, &work_dbuff);
break;
case FR_TYPE_INT64:
FR_DBUFF_OUT_RETURN(&dst->vb_int64, &work_dbuff);
break;
case FR_TYPE_FLOAT32:
FR_DBUFF_OUT_RETURN(&dst->vb_float32, &work_dbuff);
break;
case FR_TYPE_FLOAT64:
FR_DBUFF_OUT_RETURN(&dst->vb_float64, &work_dbuff);
break;
case FR_TYPE_ATTR:
if (!enumv) {
fr_strerror_const("No enumv (i.e. root) passed to fr_value_box_from_network for type 'attribute'");
return 0;
}
/*
* Decode the number, and see if we can create a
* matching attribute.
*/
{
unsigned int num;
uint8_t num8;
uint16_t num16;
uint32_t num32;
switch (enumv->flags.length) {
case 1:
FR_DBUFF_OUT_RETURN(&num8, &work_dbuff);
num = num8;
break;
case 2:
FR_DBUFF_OUT_RETURN(&num16, &work_dbuff);
num = num16;
break;
case 4:
FR_DBUFF_OUT_RETURN(&num32, &work_dbuff);
num = num32;
break;
default:
fr_strerror_const("Unsupported parent length");
return 0;
}
dst->vb_attr = fr_dict_attr_child_by_num(enumv, num);

The comment in this function says:

 *	- >= 0 The number of bytes consumed.
 *	- <0 - The negative offset where the error occurred.

but these error cases return 0;.

This bug was found with ZeroPath.

Log output from the FreeRADIUS daemon

n/a

Relevant log output from client utilities

No response

Backtrace from LLDB or GDB

Metadata

Metadata

Assignees

No one assigned

    Labels

    defectcategory: a defect or misbehaviour

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions