Skip to content

Heap buffer overflows in RT-Thread finsh #8286

Open

Description

@0xdea

Hi,

I would like to report other potential vulnerabilities in the current version of RT-Thread. Please let me know if you plan to ask for a CVE ID in case the vulnerabilities are confirmed. I'm available if you need further clarifications.

Potential heap buffer overflows in RT-Thread finsh

Summary

I spotted some potential heap buffer overflow vulnerabilities at the following locations in the RT-Thread finsh source code:
https://github.com/RT-Thread/rt-thread/blob/master/components/finsh/msh_file.c#L346
https://github.com/RT-Thread/rt-thread/blob/master/components/finsh/msh_file.c#L865
https://github.com/RT-Thread/rt-thread/blob/master/components/finsh/msh.c#L654

Details

Unbounded rt_sprintf() in the directory_delete_for_msh() function could lead to a heap buffer overflow at the marked line:

static void directory_delete_for_msh(const char *pathname, char f, char v)
{
    DIR *dir = NULL;
    struct dirent *dirent = NULL;
    char *full_path;

    if (pathname == RT_NULL)
        return;

    full_path = (char *)rt_malloc(DFS_PATH_MAX);
    if (full_path == RT_NULL)
        return;

    dir = opendir(pathname);
    if (dir == RT_NULL)
    {
        if (f == 0)
        {
            rt_kprintf("cannot remove '%s'\n", pathname);
        }
        rt_free(full_path);
        return;
    }

    while (1)
    {
        dirent = readdir(dir);
        if (dirent == RT_NULL)
            break;
        if (rt_strcmp(".", dirent->d_name) != 0 &&
                rt_strcmp("..", dirent->d_name) != 0)
        {
            rt_sprintf(full_path, "%s/%s", pathname, dirent->d_name); /* VULN: full_path is DFS_PATH_MAX bytes (only 256 bytes), potentially not enough to accommodate pathname + dirent->d_name */
            if (dirent->d_type != DT_DIR)
            {
                if (unlink(full_path) != 0)
                {
                    if (f == 0)
                        rt_kprintf("cannot remove '%s'\n", full_path);
                }
                else if (v)
                {
                    rt_kprintf("removed '%s'\n", full_path);
                }
            }
            else
            {
                directory_delete_for_msh(full_path, f, v);
            }
        }
    }
    closedir(dir);
    rt_free(full_path);
    if (rmdir(pathname) != 0)
    {
        if (f == 0)
            rt_kprintf("cannot remove '%s'\n", pathname);
    }
    else if (v)
    {
        rt_kprintf("removed directory '%s'\n", pathname);
    }
}

Unbounded rt_sprintf() in the directory_setattr() function could lead to a heap buffer overflow at the marked line:

static void directory_setattr(const char *pathname, struct dfs_attr *attr, char f, char v)
{
    DIR *dir = NULL;
    struct dirent *dirent = NULL;
    char *full_path;

    if (pathname == RT_NULL)
        return;

    full_path = (char *)rt_malloc(DFS_PATH_MAX);
    if (full_path == RT_NULL)
        return;

    dir = opendir(pathname);
    if (dir == RT_NULL)
    {
        if (f == 0)
        {
            rt_kprintf("cannot open '%s'\n", pathname);
        }
        rt_free(full_path);
        return;
    }

    while (1)
    {
        dirent = readdir(dir);
        if (dirent == RT_NULL)
            break;
        if (rt_strcmp(".", dirent->d_name) != 0 &&
            rt_strcmp("..", dirent->d_name) != 0)
        {
            rt_sprintf(full_path, "%s/%s", pathname, dirent->d_name); /* VULN: full_path is DFS_PATH_MAX bytes (only 256 bytes), potentially not enough to accommodate pathname + dirent->d_name */
            if (dirent->d_type == DT_REG)
            {
                if (dfs_file_setattr(full_path, attr) != 0)
                {
                    if (f == 0)
                    {
                        rt_kprintf("'%s' setattr failed, no such file or directory\n", full_path);
                    }
                }
                else if (v)
                {
                    rt_kprintf("'%s' setattr 0x%X\n", full_path, attr->st_mode);
                }
            }
            else if (dirent->d_type == DT_DIR)
            {
                directory_setattr(full_path, attr, f, v);
            }
        }
    }
    closedir(dir);
    rt_free(full_path);
    if (dfs_file_setattr(pathname, attr) != 0)
    {
        if (f == 0)
        {
            rt_kprintf("'%s' setattr failed, no such file or directory\n", pathname);
        }
    }
    else if (v)
    {
        rt_kprintf("'%s' setattr 0x%X\n", pathname, attr->st_mode);
    }
}

Unbounded strcpy() in the msh_auto_complete_path() function could lead to a heap buffer overflow at the marked line:

#ifdef DFS_USING_POSIX
void msh_auto_complete_path(char *path)
{
    DIR *dir = RT_NULL;
    struct dirent *dirent = RT_NULL;
    char *full_path, *ptr, *index;

    if (!path)
        return;

    full_path = (char *)rt_malloc(256);
    if (full_path == RT_NULL) return; /* out of memory */

    if (*path != '/')
    {
        getcwd(full_path, 256);
        if (full_path[rt_strlen(full_path) - 1]  != '/')
            strcat(full_path, "/");
    }
    else *full_path = '\0';

    index = RT_NULL;
    ptr = path;
    for (;;)
    {
        if (*ptr == '/') index = ptr + 1;
        if (!*ptr) break;

        ptr ++;
    }
    if (index == RT_NULL) index = path;

    if (index != RT_NULL)
    {
        char *dest = index;

        /* fill the parent path */
        ptr = full_path;
        while (*ptr) ptr ++;

        for (index = path; index != dest;)
            *ptr++ = *index++;
        *ptr = '\0';

        dir = opendir(full_path);
        if (dir == RT_NULL) /* open directory failed! */
        {
            rt_free(full_path);
            return;
        }

        /* restore the index position */
        index = dest;
    }

    /* auto complete the file or directory name */
    if (*index == '\0') /* display all of files and directories */
    {
        for (;;)
        {
            dirent = readdir(dir);
            if (dirent == RT_NULL) break;

            rt_kprintf("%s\n", dirent->d_name);
        }
    }
    else
    {
        int multi = 0;
        rt_size_t length, min_length;

        min_length = 0;
        for (;;)
        {
            dirent = readdir(dir);
            if (dirent == RT_NULL) break;

            /* matched the prefix string */
            if (strncmp(index, dirent->d_name, rt_strlen(index)) == 0)
            {
                multi ++;
                if (min_length == 0)
                {
                    min_length = rt_strlen(dirent->d_name);
                    /* save dirent name */
                    strcpy(full_path, dirent->d_name); /* VULN: if sizeof(dirent->d_name) > 256, we could overflow past the full_path buffer */
                }

                length = str_common(dirent->d_name, full_path);

                if (length < min_length)
                {
                    min_length = length;
                }
            }
        }

        if (min_length)
        {
            if (multi > 1)
            {
                /* list the candidate */
                rewinddir(dir);

                for (;;)
                {
                    dirent = readdir(dir);
                    if (dirent == RT_NULL) break;

                    if (strncmp(index, dirent->d_name, rt_strlen(index)) == 0)
                        rt_kprintf("%s\n", dirent->d_name);
                }
            }

            length = index - path;
            rt_memcpy(index, full_path, min_length);
            path[length + min_length] = '\0';

            /* try to locate folder */
            if (multi == 1)
            {
                struct stat buffer = {0};
                if ((stat(path, &buffer) == 0))
                {
                    if (S_ISDIR(buffer.st_mode))
                    {
                        strcat(path, "/");
                    }
                    else if (S_ISLNK(buffer.st_mode))
                    {
                        DIR *dir = opendir(path);
                        if (dir)
                        {
                            closedir(dir);
                            strcat(path, "/");
                        }
                    }
                }
            }
        }
    }

    closedir(dir);
    rt_free(full_path);
}
#endif /* DFS_USING_POSIX */

Impact

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

Activity

  1. changed the title [-]Potential heap buffer overflow in RT-Thread finsh[/-] [+]Potential heap buffer overflows in RT-Thread finsh[/+] on Nov 24, 2023
  2. added
    bugThis PR/issue is a bug in the current code.
    on Nov 29, 2023
  3. 0xdea commented on Dec 24, 2023

    @0xdea
    Author

    Hi, it's been one month since I reported this vulnerability, and I wanted to ask if you have any update. As standard practice, I plan to request a CVE ID for every confirmed vulnerability. I also intend to publish an advisory by February at the latest, unless there's a specific reason to postpone. Thanks!

  4. changed the title [-]Potential heap buffer overflows in RT-Thread finsh[/-] [+]Heap buffer overflows in RT-Thread finsh[/+] on Feb 2, 2024
  5. 0xdea commented on Feb 8, 2024

    @0xdea
    Author

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

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugThis PR/issue is a bug in the current code.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions