RT-Thread / RT-Thread/rt-thread

Multiple vulnerabilities in RT-Thread ymodem utility

Open
#8,291 2 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

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.

Multiple potential vulnerabilities in RT-Thread ymodem utility

Summary

I spotted some potential vulnerabilities at the following locations in the RT-Thread ymodem utility source code:
https://github.com/RT-Thread/rt-thread/blob/master/components/utilities/ymodem/ry_sy.c#L149
https://github.com/RT-Thread/rt-thread/blob/master/components/utilities/ymodem/ry_sy.c#L205
https://github.com/RT-Thread/rt-thread/blob/master/components/utilities/ymodem/ry_sy.c#L225

Details

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

static enum rym_code _rym_send_begin(
    struct rym_ctx *ctx,
    rt_uint8_t *buf,
    rt_size_t len)
{
    struct custom_ctx *cctx = (struct custom_ctx *)ctx;
    struct stat file_buf;
    char insert_0 = '\0';
    rt_err_t err;

    cctx->fd = open(cctx->fpath, O_RDONLY);
    if (cctx->fd < 0)
    {
        err = rt_get_errno();
        rt_kprintf("error open file: %d\n", err);
        return RYM_ERR_FILE;
    }
    rt_memset(buf, 0, len);
    err = stat(cctx->fpath, &file_buf);
    if (err != RT_EOK)
    {
        rt_kprintf("error open file.\n");
        return RYM_ERR_FILE;
    }

    const char *fdst = _get_path_lastname(cctx->fpath);
    if(fdst != cctx->fpath)
    {
        fdst = dfs_normalize_path(RT_NULL, fdst);
        if (fdst == RT_NULL)
        {
            return RYM_ERR_FILE;
        }
    }

    rt_sprintf((char *)buf, "%s%c%d", fdst, insert_0, file_buf.st_size); /* VULN: if we can make fdst large enough, there's no bound checking */

    return RYM_CODE_SOH;
}

Lack of NUL-termination in the rym_download_file() function at the marked line:

static rt_err_t rym_download_file(rt_device_t idev,const char *file_path)
{
    rt_err_t res;
    struct custom_ctx *ctx = rt_calloc(1, sizeof(*ctx));

    if (!ctx)
    {
        rt_kprintf("rt_malloc failed\n");
        return -RT_ENOMEM;
    }
    ctx->fd = -1;
    rt_strncpy(ctx->fpath, file_path, DFS_PATH_MAX); /* VULN: unterm if file_path is at least DFS_PATH_MAX bytes (comes from argv); it might cause infoleak or memory corruption */
    RT_ASSERT(idev);
    res = rym_recv_on_device(&ctx->parent, idev, RT_DEVICE_OFLAG_RDWR | RT_DEVICE_FLAG_INT_RX,
                             _rym_recv_begin, _rym_recv_data, _rym_recv_end, 1000);
    rt_free(ctx);

    return res;
}

Lack of NUL-termination in the rym_upload_file() function at the marked line:

static rt_err_t rym_upload_file(rt_device_t idev, const char *file_path)
{
    rt_err_t res = 0;

    struct custom_ctx *ctx = rt_calloc(1, sizeof(*ctx));
    if (!ctx)
    {
        rt_kprintf("rt_malloc failed\n");
        return -RT_ENOMEM;
    }
    ctx->fd = -1;
    rt_strncpy(ctx->fpath, file_path, DFS_PATH_MAX); /* VULN: unterm if file_path is at least DFS_PATH_MAX bytes (comes from argv); it might cause infoleak or memory corruption */
    RT_ASSERT(idev);
    res = rym_send_on_device(&ctx->parent, idev,
                             RT_DEVICE_OFLAG_RDWR | RT_DEVICE_FLAG_INT_RX,
                             _rym_send_begin, _rym_send_data, _rym_send_end, 1000);
    rt_free(ctx);

    return res;
}
Impact

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

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 with components/utilities/ymodem/ry_sy.c at lines 149, 205, and 225, then trace the callers of _rym_send_begin(), rym_download_file(), and rym_upload_file(). Verify whether the reported inputs can reach these paths and reproduce the claimed overflow or unterminated-string behavior. Done means the three reported cases are confirmed or disproved and validated against the relevant ymodem flows.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
embedded-iot, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.