Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack overflow in SNMP bulk request processing #1353

Open
mjurczak opened this issue Aug 17, 2020 · 1 comment
Open

Stack overflow in SNMP bulk request processing #1353

mjurczak opened this issue Aug 17, 2020 · 1 comment

Comments

@mjurczak
Copy link
Contributor

Description of defect

References:

https://github.com/contiki-ng/contiki-ng/tree/release/v4.5
https://github.com/contiki-ng/contiki-ng/tree/release/v4.4

File:

snmp-engine.c
snmp-message.c

Analysis:

Memory access out of buffer boundaries may occur if an SNMP bulk get request with number of OIDs larger than supported by the engine is received and processed.

The OIDs listed in a request are processed by snmp_message_decode() function without verification of the varbinds buffer capacity.
The varbinds memory buffer is written with the values provided in SNMP request:

buf = snmp_oid_decode_oid(buf, &buf_len, varbinds[i].oid, &oid_len);

The buffer capacity is determined by:
#define SNMP_MAX_NR_VALUES SNMP_CONF_MAX_NR_VALUES
#else
/**
* \brief Default maximum number of OIDs in one response
*/
#define SNMP_MAX_NR_VALUES 2
#endif

SNMP get bulk requests are processed by snmp_engine_get_bulk() function that allocates a local stack buffer for buffering OIDs of the requested variables.

snmp_engine_get_bulk(snmp_header_t *header, snmp_varbind_t *varbinds, uint32_t *varbinds_length)
{
snmp_mib_resource_t *resource;
uint32_t i, j, original_varbinds_length;
uint32_t oid[SNMP_MAX_NR_VALUES][SNMP_MSG_OID_MAX_LEN];
uint8_t repeater;

The stack buffer in snmp_engine_get_bulk() is populated with OIDs as a first step before any further processing of the data.

/*
* A local copy of the requested oids must be kept since
* the varbinds are modified on the fly
*/
original_varbinds_length = *varbinds_length;
for(i = 0; i < original_varbinds_length; i++) {
snmp_oid_copy(oid[i], varbinds[i].oid);
}

The varbinds_length variable value is not verified against the capacity of the temporary oid stack buffer. If the number of requested OIDs exceeds the buffer capacity a stack buffer overflow condition occurs and stack memory beyond the allocated oid buffer is overwritten with OIDs received in SNMP get bulk request.

As the OIDs are supplied in the request content it may be possible to alter the return address from the snmp_engine_get_bulk() function. If the target architecture uses common addressing space for program and data memory (which is common in IoT devices) it may also be possible to supply code in the SNMP request payload and redirect the execution path to the injected code by modification of the return address.

Type:

  • Out-of-bounds memory write
  • Stack memory overwrite
  • Return address alteration
  • Remote altering of code execution path
  • Remote executable code injection
  • Remote code execution

Result:

  • Memory corruption
  • Remote code execution

Target(s) affected by this defect ?

  • contiki-ng v4.5
  • contiki-ng v4.4

Fix

Rudimentary fix to address the most critical aspect of the issue:
https://github.com/mjurczak/contiki-ng/tree/bugfix/snmp-engine

How is this defect reproduced ?

An example SNMP request causing stack overwrite:

306102010104067075626C6963A554020431D065A702010402010A3046300C06082B060102010102000500300C06082B060102010102010500300C06082B0601
02010102020500300C06082B060102010102030500300C06082B060102010102040500
@g-oikonomou
Copy link
Member

@Yagoor @mjurczak: Am I right to assume that this has been fixed in #1355 and/or #1397? Can we close?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants