For the get and set options of both at86rf2xx and kw2xrf (and maybe more devices, but I did not care to look), the width of some scalar values is assumed to be like an array (the check is not done with a != operator but with <, >, <= or >=) but than set as a scalar value by way of pointer dereferencing. I noticed it first for NETOPT_MAX_PACKET_SIZE, but I suspect there are more:
case NETOPT_MAX_PACKET_SIZE:
if (len < sizeof(int16_t)) {
return -EOVERFLOW;
}
*((uint16_t *)value) = KW2XRF_MAX_PKT_LENGTH - _MAX_MHR_OVERHEAD;
return sizeof(uint16_t);
While for unsigned numbers on little-endian platforms this shouldn't be a problem, signed numbers or on big endian platform the user might get returned a non-error, but the value might not be as expected. E.g. if in the above example len == sizeof(uint32) on the setting of a value results in a memory region like this:
0 1 2 3
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|----------value_space----------|0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0|
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
On a little-endian platform uint32_t be converted correctly, since they are in the most significant place, on big-endian platforms they would however be in the most significant place, making the number really large. The other way around it would be for the set case as on big-endian platforms the 16 most significant bits would be taken, which are 0 here.
To fix this constructs like the one check should be transferred to a
if (len != sizeof(uint16_t)) {
return -EOVERFLOW;
}
or assert(len == sizeof(uint16_t)); (which would be preferred anyway).
A subset of this issue was already solved in #7488, but there are more problems.
For the
getandsetoptions of bothat86rf2xxandkw2xrf(and maybe more devices, but I did not care to look), the width of some scalar values is assumed to be like an array (the check is not done with a!=operator but with<,>,<=or>=) but than set as a scalar value by way of pointer dereferencing. I noticed it first forNETOPT_MAX_PACKET_SIZE, but I suspect there are more:While for unsigned numbers on little-endian platforms this shouldn't be a problem, signed numbers or on big endian platform the user might get returned a non-error, but the value might not be as expected. E.g. if in the above example
len == sizeof(uint32)on the setting of a value results in a memory region like this:On a little-endian platform uint32_t be converted correctly, since they are in the most significant place, on big-endian platforms they would however be in the most significant place, making the number really large. The other way around it would be for the
setcase as on big-endian platforms the 16 most significant bits would be taken, which are 0 here.To fix this constructs like the one check should be transferred to a
or
assert(len == sizeof(uint16_t));(which would be preferred anyway).A subset of this issue was already solved in #7488, but there are more problems.