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

Out of bounds memory read/write in SNMP agent #1351

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

Out of bounds memory read/write in SNMP agent #1351

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-oid.c

Analysis:

Memory access out of buffer boundaries may occur if an SNMP request with malformed OID is processed using snmp_oid_cmp_oid(), snmp_oid_encode_oid() or snmp_oid_decode_oid() when parsing a request or assembling a response.

Buffers dedicated to storing OID values used by the SNMP implementation are fixed-size with predefined length configurable at compile time:

#ifdef SNMP_CONF_MSG_OID_MAX_LEN
/**
* \brief Configurable maximum number of IDs in one OID
*/
#define SNMP_MSG_OID_MAX_LEN SNMP_CONF_MSG_OID_MAX_LEN
#else
/**
* \brief Default maximum number of IDs in one OID
*/
#define SNMP_MSG_OID_MAX_LEN 16
#endif

All snmp_varbind_t type variables encapsulate an OID buffer of the predefined length:

uint32_t oid[SNMP_MSG_OID_MAX_LEN];

With the following occurences of OID buffers allocation in the SNMP code:

Static allocation in .bss:

static snmp_varbind_t varbinds[SNMP_MAX_NR_VALUES];

Stack allocation:

uint32_t oid[SNMP_MAX_NR_VALUES][SNMP_MSG_OID_MAX_LEN];

uint8_t *snmp_oid_decode_oid(uint8_t *buf, uint32_t *buff_len, uint32_t *oid, uint32_t *oid_len)
In OID decoding function :

snmp_oid_decode_oid(uint8_t *buf, uint32_t *buff_len, uint32_t *oid, uint32_t *oid_len)

the length of OID data is not verified against the input buffer remaining length. The value in the buffer is trusted to indicate correct number of following OID data bytes:

buf = snmp_ber_decode_length(buf, buff_len, &len);
if(buf == NULL) {
return NULL;
}

The result of decoding is used for computing pointer to the end of buffer, which may result in buf_end variable pointing beyond the input message end:

buf_end = buf + len;

as the BER length decoding function does not perform decoded length validation against the input buffer remaining length:

snmp_ber_decode_length(unsigned char *buff, uint32_t *buff_len, uint8_t *length)
{
*length = *buff++;
(*buff_len)--;
return buff;
}

In addition to the above, the check of provided oid output buffer in snmp_oid_decode_oid:

--(*oid_len);
if(*oid_len == 0) {
return NULL;
}

may not protect against the overflow when called from SNMP request parsing function:
buf = snmp_oid_decode_oid(buf, &buf_len, varbinds[i].oid, &oid_len);

due to the fact that oid_len variable remains uninitialized during the call to the function.

As a result of the above, memory beyond provided input buffer may be read-accessed and memory beyond target buffer capacity may be written if an OID with length larger than SNMP_MSG_OID_MAX_LEN is present in SNMP request message. As the content of write operation is directly provided in the SNMP request, it may be possible to overwrite stack or .bss memory regions with arbitrary content provided in a request with OID length exceeding SNMP_MSG_OID_MAX_LEN limit.

void snmp_oid_copy(uint32_t *dst, uint32_t *src)
unsigned char *snmp_oid_encode_oid(unsigned char *out, uint32_t *out_len, uint32_t *oid)
OID copy and encode functions rely on the fact that the decoded OID has been terminated with a null-termination value without verification of the targete buffers capacity. This makes both of the functions vulnerable to similar out-ouf-bounds writes.

Type:

  • Out-of-bounds memory read
  • Out-of-bounds memory write

Result:

  • Memory corruption
  • Memory write to areas after the target buffer end with arbitrary data

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 ?

Example code demonstrating memory buffer overflow when decoding OID:

#include <stdint.h>
#include <string.h>

#include "snmp-oid.h"

static uint8_t test_oid1[] = {0x06,0x08,0x2B,0x06,0x01,0x02,0x01,0x01,0x02,0x00};
static uint8_t test_oid2[] = {0x06,0x27,0x2B,0x06,
                                        0x01,0x01,0x01,0x01,0x01,0x01,0x01,0x01,
                                        0x01,0x01,0x01,0x01,0x01,0x01,0x01,0x01,
                                        /*Buffer capacity limit*/
                                        0x86,0xE3,0xB1,0xCA,0xC8,0x86,0xE3,0xB1,0xCA,0xC8,
                                        0x86,0xE3,0xB1,0xCA,0xC8,0x86,0xE3,0xB1,0xCA,0xC8, 0x00};

static uint32_t test_oid1_len = sizeof(test_oid1);
static uint32_t test_oid2_len = sizeof(test_oid2);

void print_hex(char *data, size_t data_len)
{
    while(data_len--)
    {
        printf("%02hhX", *data++);
    }
    printf("\r\n");
}

int main(int argc, const char* argv[])
{
    uint8_t *ret;
    uint32_t oid1_len = 0;
    uint32_t oid2_len = 0;
    uint32_t oid1[SNMP_MSG_OID_MAX_LEN];
    uint32_t oid2[SNMP_MSG_OID_MAX_LEN];
    char text[] = "This is a very important stack variable.";


    printf("VALID OID:\r\n");
    ret = snmp_oid_decode_oid(test_oid1, &test_oid1_len, oid1, &oid1_len);
    printf("returned ptr = %p\r\n", ret);
    printf("Decoded oid length: %d\r\n", oid1_len);
    print_hex(text, sizeof(text));

    printf("BAD OID:\r\n");
    ret = snmp_oid_decode_oid(test_oid2, &test_oid2_len, oid2, &oid2_len);
    printf("returned ptr = %p\r\n", ret);
    printf("Decoded oid length: %d\r\n", oid2_len);
    print_hex(text, sizeof(text));

    return 0;
}
@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