[Bug] RT-Thread AB32 ADC uses an unchecked channel as a register index

未关闭 适合新手
#11,948 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

@usualtreee 已经在做这个了。

开始于 2026年10月31日。

  • #11962 来自 @usualtreee —— 未关闭

评估

难度
2/5
预计耗时
1-3 小时
新手友好度
72/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃
技术栈
c

调研方向

从 bsp/bluetrum/libraries/hal_drivers/drv_adc.c 中的 AB32 ADC 回调以及 components/drivers/include/drivers/adc.h 中的回调定义开始。追踪通道值如何传递到 HAL 转换和 SADCDAT 寄存器指针;在返回采样值之前,检查十六个寄存器的范围和转换结果。完成标准是拒绝不受支持的通道(包括负数的内部通道常量),并且有效通道在转换和读取时使用同一个经过检查的值。

由索引模型根据 Issue 内容生成。

描述

bug Component component: drivers
RT-Thread Version

v5.3.0

Affected area

Device drivers

Hardware/BSP vendor

Not applicable / Other

Architecture

Not applicable / Other

Board and hardware details

Bluetrum AB32 ADC

Develop Toolchain

Other

Describe the bug

RT-Thread AB32 ADC uses an unchecked channel as a register index

Affected version

RT-Thread v5.3.0.

Describe the bug

The Bluetrum AB32 ADC driver maps a channel before starting a conversion, then uses the original channel to index the ADC data register pointer. Channels outside the sixteen SADCDAT registers are not rejected.

rt_adc_read() forwards the channel without a range check. ab32_adc_get_channel() recognizes cases 0 through 15 and leaves every other value as 0, so the HAL start uses a fallback channel instead of returning an error. ab32_get_adc_value() then reads ab32_adc_handler[channel] with the original value:

ab32_adc_handler = device->parent.user_data;
hal_adc_start(ab32_adc_get_channel(channel));
hal_adc_poll_for_conversion(1000);
*value = ab32_adc_handler[channel];

Registration stores (hal_sfr_t)&SADCDAT0 as that pointer. SADCDAT0 through SADCDAT15 are sixteen contiguous registers. Channel 16 therefore selects the word after SADCDAT15.

The public ADC callbacks take rt_int8_t channel. Both AB32 callbacks take rt_uint32_t channel. The two signatures should match struct rt_adc_ops; changing only the types still leaves the index unchecked.

Locations:

bsp/bluetrum/libraries/hal_drivers/drv_adc.c:53
bsp/bluetrum/libraries/hal_drivers/drv_adc.c:112
bsp/bluetrum/libraries/hal_drivers/drv_adc.c:124
bsp/bluetrum/libraries/hal_drivers/config/adc_config.h:24
bsp/bluetrum/libraries/hal_libraries/bmsis/include/ab32vg1.h:203
components/drivers/include/drivers/adc.h:74
components/drivers/misc/adc.c:118

Threat model

The boundary is a caller of the public ADC API. This report does not treat a CAN, network, or physical-bus peer as the attacker. The value that caller controls is the channel argument of rt_adc_read(dev, channel). dev only selects the ADC instance. The register base is fixed at registration as SADCDAT0; the caller does not supply that pointer.

channel has type rt_int8_t, so the values defined by the API are -128 through 127. Channels 0 through 15 name SADCDAT0 through SADCDAT15. Channels 16 through 127, and the negative constants RT_ADC_INTERN_CH_TEMPER, RT_ADC_INTERN_CH_VREF, and RT_ADC_INTERN_CH_VBAT (-1, -2, and -3), fall outside that window and are still accepted. The AB32 callback declares the same argument as rt_uint32_t. That mismatch does not give the caller a 32-bit index; the controlled input remains the 8-bit channel.

With RT_USING_FINSH, adc read <channel> passes atoi(argv[2]) into rt_adc_read(). The shell is an entry point only when an untrusted user can reach it. A product task that forwards an untrusted channel into the same API is the same boundary. A trusted task passing a bad constant is a driver defect in its own firmware, not a new privilege boundary.

Steps to reproduce

I have not reproduced this on AB32 hardware. The reduced check keeps the fallback mapping and the original-channel index, using a sixteen-word array in place of the SADCDAT window.

#include <stdint.h>

static volatile uint32_t sadcdat[16];

static uint32_t map_channel(uint32_t channel)
{
    uint32_t mapped = 0;

    switch (channel) {
    case 0: case 1: case 2: case 3: case 4: case 5: case 6: case 7:
    case 8: case 9: case 10: case 11: case 12: case 13: case 14: case 15:
        mapped = channel;
        break;
    }
    return mapped;
}

static void convert(uint32_t channel, uint32_t *value)
{
    (void)map_channel(channel);
    *value = sadcdat[channel];
}

int main(void)
{
    uint32_t value = 0;

    convert(16, &value);
    return (int)value;
}

Compile it with a bounds sanitizer, for example:

clang -fsanitize=address,undefined -g -O1 repro.c -o repro && ./repro

Rejecting a channel outside 0..15 before the index avoids the report. The same rejection should apply to the negative internal-channel constants.

Relevant log output

runtime error: index 16 out of bounds for type 'volatile uint32_t[16]'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior

Impact

A caller that can pass an ADC channel to rt_adc_read() can select outside the sixteen data registers. With RT_USING_FINSH, the adc shell command passes parsed text to that API. Whether that console is exposed, and whether the resulting address is readable or faulting, depends on the product.

This is an unchecked peripheral-register index. The reduced array check does not show an arbitrary RAM read or a physical-device crash.

Suggested fix

Match both callbacks to struct rt_adc_ops. Accept only channels that have a defined SADCDAT register, and use that same checked value for the HAL start and the data read. Do not substitute channel 0 for an unsupported value. Check the conversion result before returning a sample.

Other additional context

No response

主要语言
C
星标
12.3k
派生
5.5k
平均合并
4 天 9 小时
30 天内合并 PR
38

环境准备

在 Codespaces 中打开

在浏览器里用你自己的 GitHub 账号启动这个项目的开发容器。

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

RT-Thread/rt-thread 的其他 Issue

查看 RT-Thread/rt-thread 的全部 Issue

相似的 Issue

更多 C Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。