Heap buffer overflows in RT-Thread finsh

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.

Start by reviewing components/finsh/msh_file.c in directory_delete_for_msh() and directory_setattr(), then components/finsh/msh.c in msh_auto_complete_path(), focusing on the reported unbounded formatting and copying. Exercise these entry points with paths or directory names exceeding the allocated buffers; done means the reported heap overflows are confirmed or disproved and the affected operations are safe.