Buffer overflow in RT-Thread dfs_v2 romfs filesystem

[0xdea]

Hi,

I would like to report a potential vulnerability in the current version of RT-Thread. As directed, I'm opening an issue here. Please let me know if you plan to ask for a CVE ID in case the vulnerability is confirmed. I'm available if you need further clarifications.

Potential buffer overflow in RT-Thread dfs_v2 romfs filesystem

Summary

I spotted a potential buffer overflow vulnerability at the following location in the RT-Thread dfs_v2 romfs filesystem source code:
https://github.com/RT-Thread/rt-thread/blob/master/components/dfs/dfs_v2/filesystems/romfs/dfs_romfs.c#L353-L355

Details

Lack of length check in the dfs_romfs_getdents() function could lead to a buffer overflow at the marked line:

static int dfs_romfs_getdents(struct dfs_file *file, struct dirent *dirp, uint32_t count)
{
    rt_size_t index;
    const char *name;
    struct dirent *d;
    struct romfs_dirent *dirent, *sub_dirent;

    dirent = (struct romfs_dirent *)file->vnode->data;
    if (check_dirent(dirent) != 0)
    {
        return -EIO;
    }
    RT_ASSERT(dirent->type == ROMFS_DIRENT_DIR);

    /* enter directory */
    dirent = (struct romfs_dirent *)dirent->data;

    /* make integer count */
    count = (count / sizeof(struct dirent));
    if (count == 0)
    {
        return -EINVAL;
    }

    index = 0;
    for (index = 0; index < count && file->fpos < file->vnode->size; index++)
    {
        d = dirp + index;

        sub_dirent = &dirent[file->fpos];
        name = sub_dirent->name;

        /* fill dirent */
        if (sub_dirent->type == ROMFS_DIRENT_DIR)
            d->d_type = DT_DIR;
        else
            d->d_type = DT_REG;

        d->d_namlen = rt_strlen(name);
        d->d_reclen = (rt_uint16_t)sizeof(struct dirent);
        rt_strncpy(d->d_name, name, rt_strlen(name) + 1); /* VULN: buffer overflow if rt_strlen(name) is larger than sizeof(d->d_name) due to missing length check */

        /* move to next position */
        ++ file->fpos;
    }

    return index * sizeof(struct dirent);
}

Please note that dfs_v1 romfs source code at https://github.com/RT-Thread/rt-thread/blob/master/components/dfs/dfs_v1/filesystems/romfs/dfs_romfs.c#L335-L340 is not affected, because the string copy operation is implemented differently:

        len = rt_strlen(name);
        RT_ASSERT(len <= RT_UINT8_MAX);
        d->d_namlen = (rt_uint8_t)len;
        d->d_reclen = (rt_uint16_t)sizeof(struct dirent);
        rt_strncpy(d->d_name, name, DFS_PATH_MAX);

Even if the assertion is compiled-out in production code, len is not used for the copy operation anyway. Therefore, unless DFS_PATH_MAX is larger than sizeof(d->d_name), this code should be safe.

Impact

If the unchecked input above is confirmed to be attacker-controlled and crossing a security boundary, the impact of the reported buffer overflow vulnerability could range from denial of service to arbitrary code execution.

[mysterywolf]

Thanks for report this, this PR has been fixed the issue #8278

[0xdea]

Thank you for letting me know! I've opened some other issues related to different vulnerabilities. I'm available if you need any clarifications. For any confirmed vulnerability (including this one), once they're fixed I'm planning to request a CVE ID from Mitre. Let me know in case you prefer to handle this yourself.

[0xdea]

Hi there, CVE-2024-24335 was assigned to this vulnerability. I'm planning to publish my security advisory and writeup on March 5th. Thanks.