The SNMPv3 USM parser reserves no room for the null terminator it writes, so a
64-byte msgUserName stores one byte past the end of a 64-byte array.
What happens
_nx_snmp_utility_octet_get() copies an octet string and rejects anything longer
than the bound it is given (addons/snmp/nxd_snmp.c:14797):
if (total > max_octet_length)
total == max_octet_length is accepted, which is correct: the function copies, it
does not terminate.
The USM path passes the full array size as that bound
(addons/snmp/nxd_snmp.c:17513):
length = _nx_snmp_utility_octet_get(buffer_ptr, agent_ptr -> nx_snmp_agent_v3_security_user_name,
sizeof(agent_ptr -> nx_snmp_agent_v3_security_user_name), &(agent_ptr -> nx_snmp_agent_v3_security_user_name_size), buffer_length);
and then terminates the result in place (addons/snmp/nxd_snmp.c:17538):
agent_ptr -> nx_snmp_agent_v3_security_user_name[agent_ptr -> nx_snmp_agent_v3_security_user_name_size] = 0;
NX_SNMP_MAX_USER_NAME is 64, so a 64-byte user name makes that a write to
nx_snmp_agent_v3_security_user_name[64] — one past the array, which is
undefined behaviour.
Effect
In NX_SNMP_AGENT the array is immediately followed by
nx_snmp_agent_v3_security_user_name_size, 4-aligned with no padding, so the byte
lands inside the same control block rather than outside the allocation. On a
little-endian target it clears the low byte of the size, taking 64 to 0, and the
request is then handled as if the user name were empty. The response is the same
discovery report a client gets by sending an empty msgUserName. On a big-endian
target the byte written is already zero and behaviour does not change.
So the practical consequence is small — but the write is out of bounds of the
array object either way, and it is reachable from an unauthenticated packet:
SNMPv3 is enabled by default at agent create, and the user name is parsed before
it is matched against any configured user.
Suggested fix
Leave room for the terminator at the call site, the way
_nx_snmp_utility_community_get() already does with
total > (NX_SNMP_MAX_USER_NAME-1):
/* Get the security username, leaving room for the null terminator. */
length = _nx_snmp_utility_octet_get(buffer_ptr, agent_ptr -> nx_snmp_agent_v3_security_user_name,
sizeof(agent_ptr -> nx_snmp_agent_v3_security_user_name) - 1,
&(agent_ptr -> nx_snmp_agent_v3_security_user_name_size), buffer_length);
A longer name is then rejected as an invalid packet. That is safe with respect to
the protocol: RFC 3414 caps msgUserName at 32 octets, so the 64-byte buffer is
already generous.
Please do not tighten the check inside _nx_snmp_utility_octet_get() to
total >= max_octet_length instead. That would also reject legitimate input at
the other call sites — NX_SNMP_MAX_CONTEXT_STRING is 32 and RFC 3411 allows a
32-byte snmpEngineID, which is exactly the maximum.
This is the only affected caller. The engine ID, authentication and privacy
parameters use the same helper but do not null-terminate, and the v1/v2c
community string path already reserves the extra byte.
Suggested tests
- 63-byte
msgUserName is accepted and the terminator stays in bounds.
- 64-byte
msgUserName is rejected, nx_snmp_agent_invalid_packets increments and
the packet is released.
- An empty
msgUserName with an empty engine ID still produces the discovery
report.
- A 32-byte engine ID is still accepted, guarding against the
>= regression
above.
The SNMPv3 USM parser reserves no room for the null terminator it writes, so a
64-byte
msgUserNamestores one byte past the end of a 64-byte array.What happens
_nx_snmp_utility_octet_get()copies an octet string and rejects anything longerthan the bound it is given (
addons/snmp/nxd_snmp.c:14797):total == max_octet_lengthis accepted, which is correct: the function copies, itdoes not terminate.
The USM path passes the full array size as that bound
(
addons/snmp/nxd_snmp.c:17513):and then terminates the result in place (
addons/snmp/nxd_snmp.c:17538):NX_SNMP_MAX_USER_NAMEis 64, so a 64-byte user name makes that a write tonx_snmp_agent_v3_security_user_name[64]— one past the array, which isundefined behaviour.
Effect
In
NX_SNMP_AGENTthe array is immediately followed bynx_snmp_agent_v3_security_user_name_size, 4-aligned with no padding, so the bytelands inside the same control block rather than outside the allocation. On a
little-endian target it clears the low byte of the size, taking 64 to 0, and the
request is then handled as if the user name were empty. The response is the same
discovery report a client gets by sending an empty
msgUserName. On a big-endiantarget the byte written is already zero and behaviour does not change.
So the practical consequence is small — but the write is out of bounds of the
array object either way, and it is reachable from an unauthenticated packet:
SNMPv3 is enabled by default at agent create, and the user name is parsed before
it is matched against any configured user.
Suggested fix
Leave room for the terminator at the call site, the way
_nx_snmp_utility_community_get()already does withtotal > (NX_SNMP_MAX_USER_NAME-1):A longer name is then rejected as an invalid packet. That is safe with respect to
the protocol: RFC 3414 caps
msgUserNameat 32 octets, so the 64-byte buffer isalready generous.
Please do not tighten the check inside
_nx_snmp_utility_octet_get()tototal >= max_octet_lengthinstead. That would also reject legitimate input atthe other call sites —
NX_SNMP_MAX_CONTEXT_STRINGis 32 and RFC 3411 allows a32-byte
snmpEngineID, which is exactly the maximum.This is the only affected caller. The engine ID, authentication and privacy
parameters use the same helper but do not null-terminate, and the v1/v2c
community string path already reserves the extra byte.
Suggested tests
msgUserNameis accepted and the terminator stays in bounds.msgUserNameis rejected,nx_snmp_agent_invalid_packetsincrements andthe packet is released.
msgUserNamewith an empty engine ID still produces the discoveryreport.
>=regressionabove.