OpenBIOS ELF Loader Buffer Overflow

Continuing to look at OpenBIOS. Here is a classic integer overflow leading to a heap based buffer overflow. nhdr->n_descsz is 32-bits. So we on x86 (32-bit), if we make it UINT_MAX, we cause ob_malloc to allocate 0 bytes. This returns a pointer. The memcpy causes memory corruption. Even if ob_calloc returned NULL, there is no error checking.

            if (nhdr->n_namesz==sizeof(ELF_NOTE_BOOT)
                    && memcmp(name, ELF_NOTE_BOOT, sizeof(ELF_NOTE_BOOT))==0) {
                if (nhdr->n_type == EIN_PROGRAM_NAME) {
                    image_name = ob_calloc(1, nhdr->n_descsz + 1);
                    memcpy(image_name, desc, nhdr->n_descsz);
                }

And to see what ob_calloc does:

static void *ob_calloc(size_t nmemb, size_t size)
{
    size_t alloc_size = nmemb * size;
    void *mem;

    if (alloc_size < nmemb || alloc_size < size) {
        printf("calloc overflow: %u, %u\n", nmemb, size);
        return NULL;
    }

    mem = malloc(alloc_size);
    memset(mem, 0, alloc_size);

    return mem;
}

And to verify that malloc(0) returns a pointer, we have:

void *malloc(int size)
{
        void *ret=(void *)0;
        if(memsize>=size) {
                memsize-=size;
                ret=memptr;
                memptr = (void *)((unsigned long)memptr + size);
        }
        return ret;
}

unmass Buffer Overflow

unmass is a package in Linux (e.g., Ubunut) to "ëxtract game archive files"

        void FillListSorted( e_sort sorttype );

        CWndSize        SizeCtrls;

        char    ProgramPath[ 512 ], TempDir[ 512 ]; // no end slashes

        int             NoExtInCombo;

        int             ArchiveOpened;

};

Ok.. so ProgramPath and TempDir are both 512 bytes.

BOOL CUnmasswDlg::OnInitDialog()
{
        CDialog::OnInitDialog();

        icon = LoadIcon( AfxGetInstanceHandle(), MAKEINTRESOURCE( IDR_MAINFRAME 
) );
        SetIcon( icon, true );          // Set big icon
        SetIcon( icon, false );         // Set small icon


        ArchiveOpened = 0;

        int             i;

        GetModuleFileName( NULL, ProgramPath, 512 );
        i = strlen( ProgramPath ) - 1;
        while (( ProgramPath[ i ] != '\\' ) && ( ProgramPath[ i ] != '/' ))
                i--;
        ProgramPath[ i ] = 0;

        strcpy( TempDir, ProgramPath );
        strcat( TempDir, "\\TEMP" );

TempDir can have more than 512 bytes written to it. A simple buffer overflow.

strncat Buffer Overflow in OpenBIOS

It's easy to get strncat wrong. Here is an example from OpenBIOS.

int load_plugin(const char *plugin_name)
{
        void *handle;
        char *error;
        char path[PATHSIZE];

        int (*init_plugin) (void);
        char **deps;
        char **plugin_info;
        plugin_t *p;

        if (is_loaded(plugin_name)) {
                printf("Plugin %s already loaded.\n", plugin_name);
                return 0;
        }

        strncpy(path, PLUGINDIR, PATHSIZE);
        strncat(path, "/plugin_", PATHSIZE);
        strncat(path, plugin_name, PATHSIZE);
        strncat(path, ".so", PATHSIZE);

What does setting the size field in strncat to PATHSIZE do? It won't prevent the destination string from exceeding the size PATHSIZE. It will prevent the source string being copied to copy at most PATHSIZE characters. In the above code, if the plugin_name is near PATHSIZE, then a buffer overflow will occur. There are more bugs like this in OpenBIOS.

strncpy again in xen

I know I'm repeating myself.. strncpy must always be explicitly NULL terminated. Here is another example from xen (utils)

static int get_name(int argc, char *argv[], char *name)
{
    ssize_t len = strlen(argv[0]);
    if ( len > XEN_LIVEPATCH_NAME_SIZE )
    {
        fprintf(stderr, "ID must be no more than %d characters.\n",
                XEN_LIVEPATCH_NAME_SIZE);
        errno = EINVAL;
        return errno;
    }
    /* Don't want any funny strings from the stack. */
    memset(name, 0, XEN_LIVEPATCH_NAME_SIZE);
    strncpy(name, argv[0], len);
    return 0;
}

There are a few other places in xen too..

Does anyone get strncpy right?

I decided to have a quick look at OpenBIOS. 5 seconds in, it's clear that strncpy is used incorrectly in a number of places. OpenBSD realised many years ago how prone to bugs strncpy was and replaced it with the non standard strlcpy.

static void
create_free_part( char *ptr, int size )
{
        nvpart_t *nvp = (nvpart_t*)ptr;
        memset( nvp, 0, size );

        strncpy( nvp->name, "777777777777", sizeof(nvp->name) );
        nvp->signature = NV_SIG_FREE;
        nvp->len_hi = (size /16) >> 8;
        nvp->len_lo = size /16;
        nvp->checksum = nvpart_checksum(nvp);
}

And more..

static int
create_nv_part( int signature, const char *name, int size )
{
        nvpart_t *p = NULL;
        int fs;

        while( next_nvpart(&p) > 0 ) {
                if( p->signature != NV_SIG_FREE )
                        continue;

                fs = nvpart_size( p );
                if( fs < size )
                        size = fs;
                p->signature = signature;
                memset( p->name, 0, sizeof(p->name) );
                strncpy( p->name, name, sizeof(p->name) );
                p->len_hi = (size>>8)/16;
                p->len_lo = size/16;
                p->checksum = nvpart_checksum(p);
                if( fs > size ) {
                        char *fp = (char*)p + size;
                        create_free_part( fp, fs-size );
                }
                return size;
        }
        printk("create-failed\n");
        return -1;
}

And more..

        intprop = get_int_property(chosen, "stdout", &proplen);
        PUSH(intprop);
        fword("get-instance-path");
        ((struct linux_romvec *)romvec)->pv_stdout = pop_fstr_copy();

        /* Get the name of the selected boot device, along with the device and u
nit number */
        prop = get_property(chosen, "bootpath", &proplen);
        strncpy(bootpathbuf, prop, proplen);
        prop = get_property(chosen, "bootargs", &proplen);
        strncpy(bootargsbuf, prop, proplen);    

        /* Set bootpath pointer used in romvec table to the bootpath */
        push_str(bootpathbuf);
        fword("pathres-resolve-aliases");
        bootpath = pop_fstr_copy();
        printk("bootpath: %s\n", bootpath);

There are a number of more cases of this simple bug, but I think the readers get the point..

Apache Nimble Bluetooth Stack

I had a quick look last night at Apache Nimble, an open source Bluetooth stack.

Here are some minor findings:

static char *
ble_gatts_flags_to_str(uint16_t flags, char *buf,

                       const char * const *names)

{
    int bit;
    bool non_empty = false;
    size_t length = 0;

    buf[0] = '\0';
    strcpy(buf, "[");
    length += 1;
    for (bit = 0; names[bit]; ++bit) {
        if (flags & (1 << bit)) {
            length += strlen(names[bit]);
            if (length + 1 >= BLE_CHR_FLAGS_STR_LEN) {
                return buf;
            }
            if (non_empty) {
                strcat(buf, "|");
                length += 1;
            }
            strcat(buf, names[bit]);
            non_empty = true;
        }
    }
    strcat(buf, "]");
    return buf;
}

The above code has an off-by-1.

There are some strncpy bugs, where strings may be left unterminated.

int bt_mesh_input_string(const char *str)
{
        BT_DBG("%s", str);

        if (!atomic_test_and_clear_bit(link.flags, WAIT_STRING)) {
                return -EINVAL;
        }

        strncpy((char *)link.auth, str, prov->input_size);

        send_input_complete();

        if (!atomic_test_bit(link.flags, HAVE_DHKEY)) {
                return 0;
        }

        if (atomic_test_and_clear_bit(link.flags, SEND_CONFIRM)) {
                send_confirm();
        }

        return 0;
}

And again,

static void input_string(u8_t *data, u16_t len)
{
        const struct mesh_input_string_cmd *cmd = (void *) data;
        u8_t status = BTP_STATUS_SUCCESS;
        u8_t str_auth[16];
        int err;

        SYS_LOG_DBG("");

        if (cmd->string_len > sizeof(str_auth)) {
                SYS_LOG_ERR("Too long input (%u chars required)", input_size);
                status = BTP_STATUS_FAILED;
                goto rsp;
        } else if (cmd->string_len < input_size) {
                SYS_LOG_ERR("Too short input (%u chars required)", input_size);
                status = BTP_STATUS_FAILED;
                goto rsp;
        }

        strncpy((char *)str_auth, (char *)cmd->string, cmd->string_len);

        err = bt_mesh_input_string((char *)str_auth);
        if (err) {
                status = BTP_STATUS_FAILED;
        }

rsp:
        tester_rsp(BTP_SERVICE_ID_MESH, MESH_INPUT_STRING, CONTROLLER_INDEX,
                   status);
}

And one more time,

struct os_mempool *
os_mempool_info_get_next(struct os_mempool *mp, struct os_mempool_info *omi)
{
    struct os_mempool *cur;

    if (mp == NULL) {
        cur = STAILQ_FIRST(&g_os_mempool_list);
    } else {
        cur = STAILQ_NEXT(mp, mp_list);
    }

    if (cur == NULL) {
        return (NULL);
    }

    omi->omi_block_size = cur->mp_block_size;
    omi->omi_num_blocks = cur->mp_num_blocks;
    omi->omi_num_free = cur->mp_num_free;
    omi->omi_min_free = cur->mp_min_free;
    strncpy(omi->omi_name, cur->name, sizeof(omi->omi_name));

    return (cur);
}

The only other use of strncpy in the code base, they write correct code.

int
ble_monitor_new_index(uint8_t bus, uint8_t *addr, const char *name)
{
    struct ble_monitor_new_index pkt;

    pkt.type = 0; /* Primary controller, we don't support other */
    pkt.bus = bus;
    memcpy(pkt.bdaddr, addr, 6);
    strncpy(pkt.name, name, sizeof(pkt.name) - 1);
    pkt.name[sizeof(pkt.name) - 1] = '\0';

    ble_monitor_send(BLE_MONITOR_OPCODE_NEW_INDEX, &pkt, sizeof(pkt));

    return 0;
}

The code in general is quite good after a quick glance.

ESP8266 Firmware Buffer Overflows

The ESP8266 is a popular IoT-style module. You've probably heard of it.

Let's look at the firmware.

Arduino/libraries/ESP8266mDNS/src/ESP8266mDNS_Legacy.cpp

Here is a classic remote buffer overflow.

void MDNSResponder::_parsePacket(){
  int i;
  char tmp;
  bool serviceParsed = false;
  bool protoParsed = false;
  bool localParsed = false;

  char hostName[255];
  uint8_t hostNameLen;

  char serviceName[32];

...

  hostNameLen = _conn_read8() % 255;
  _conn_readS(hostName, hostNameLen);
  hostName[hostNameLen] = '\0';

  if(hostName[0] == '_'){
    serviceParsed = true;
    memcpy(serviceName, hostName+1, hostNameLen);
    serviceNameLen = hostNameLen-1;
    hostNameLen = 0;
  }

This appears non exploitable due to the buffer overflow overflowing into an adjacent buffer.

There are other bugs too. The following probably will probably result in a Denial of Service.


void MDNSResponder::_parsePacket(){
  int i;
  char tmp;
  bool serviceParsed = false;
  bool protoParsed = false;
  bool localParsed = false;

  char hostName[255];
  uint8_t hostNameLen;

  char serviceName[32];
  uint8_t serviceNameLen;
  uint16_t servicePort = 0;

  char protoName[32];
  protoName[0] = 0;
  uint8_t protoNameLen = 0;

...

        _conn_readS(serviceName, tmp8);
        serviceName[tmp8] = '\0';
#ifdef DEBUG_ESP_MDNS_RX
        DEBUG_ESP_PORT.printf(" %d ", tmp8);
        for (int n = 0; n < tmp8; n++) {
          DEBUG_ESP_PORT.printf("%c", serviceName[n]);
        }
        DEBUG_ESP_PORT.println();

+++ no validation on tmp8.. serviceName is buffer of 32
+++ this isn't the DoS bug.

...

      uint16_t answerType = _conn_read16(); // Read type
      uint16_t answerClass = _conn_read16(); // Read class
      uint32_t answerTtl = _conn_read32(); // Read ttl
      uint16_t answerRdlength = _conn_read16(); // Read rdlength

+++ andwerRdLength can be < 3

      (void) answerClass;
      (void) answerTtl;

      if(answerRdlength > 255){
        if(answerType == MDNS_TYPE_TXT && answerRdlength < 1460){
          while(--answerRdlength) _conn->read();
        } else {
#ifdef DEBUG_ESP_MDNS_RX
        DEBUG_ESP_PORT.printf("Data len too long! %u\n", answerRdlength);
#endif
          _conn->flush();
          return;
        }
      }

#ifdef DEBUG_ESP_MDNS_RX
      DEBUG_ESP_PORT.printf("type: %04x rdlength: %d\n", answerType, answerRdlength);
#endif

      if (answerType == MDNS_TYPE_PTR) {
        partsCollected |= 0x01;
        _conn_readS(hostName, answerRdlength); // Read rdata
        if(hostName[answerRdlength-2] & 0xc0){
          memcpy(answerHostName, hostName+1, answerRdlength-3);

+++ int underflow when answerRdLength is < 3

          answerHostName[answerRdlength-3] = '\0';
        }