RT-Thread Version

v5.3.0

Affected area

Other / Not sure

Hardware/BSP vendor

Not applicable / Other

Architecture

Not applicable / Other

Board and hardware details

HPM ADC

Develop Toolchain

Other

Describe the bug

RT-Thread HPM ADC indexes channel state without a bounds check

Affected version

RT-Thread v5.3.0.

Describe the bug

In the checked revision, hpm_adc_enabled() indexes chn_state[16] with the raw signed channel. rt_adc_enable() does not validate that channel first.

typedef struct
{
    bool enabled;
} adc_channel_state_t;

typedef struct
{
    char *adc_name;
    struct rt_adc_device hpm_adc_device;
    bool is_adc12;
    bool adc_enabled;
    uint32_t adc_base;
    adc_channel_state_t chn_state[16];
} hpm_rtt_adc;

The enable callback reads hpm_adc->chn_state[channel].enabled and, when that entry is clear, later writes it:

if (enabled == RT_TRUE) {
    if (!hpm_adc->chn_state[channel].enabled) {
        ...
        hpm_adc->chn_state[channel].enabled = true;
        ret = init_channel_config(hpm_adc, channel);
    }
}

Channel 16 is past the sixteen entries. adc_base is the field immediately before chn_state, so a negative channel walks backward into that field before any HAL channel check. A later HAL check cannot protect the earlier array access.

Locations:

bsp/hpmicro/libraries/drivers/drv_adc.c:21
bsp/hpmicro/libraries/drivers/drv_adc.c:26
bsp/hpmicro/libraries/drivers/drv_adc.c:219
bsp/hpmicro/libraries/drivers/drv_adc.c:227
bsp/hpmicro/libraries/drivers/drv_adc.c:239
components/drivers/misc/adc.c:131

Threat model

The boundary is a caller of the public ADC API. This report does not treat a network or physical-bus peer as the attacker. The value that caller controls is the channel argument of rt_adc_enable(dev, channel). dev only selects the ADC instance. adc_base and chn_state belong to the driver object; the caller does not supply those addresses.

channel has type rt_int8_t, so the values defined by the API are -128 through 127. Entries 0 through 15 are the state array. Channel 16 is past that array. The negative constants RT_ADC_INTERN_CH_TEMPER, RT_ADC_INTERN_CH_VREF, and RT_ADC_INTERN_CH_VBAT (-1, -2, and -3) walk backward from the array into the preceding fields. rt_adc_enable() always passes RT_TRUE as the enable flag, which is the path that reads and may write the indexed state. rt_adc_disable() does not take that path.

With RT_USING_FINSH, adc enable <channel> passes atoi(argv[2]) into rt_adc_enable(). 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.

Steps to reproduce

I have not reproduced this on HPMicro hardware. The reduced check keeps adc_base immediately before a sixteen-entry state array and performs the same read-then-write.

#include <stdbool.h>
#include <stdint.h>

typedef struct { bool enabled; } adc_channel_state_t;

typedef struct {
    uint32_t adc_base;
    adc_channel_state_t chn_state[16];
} hpm_rtt_adc;

static void enable_channel(hpm_rtt_adc *adc, int8_t channel)
{
    if (!adc->chn_state[channel].enabled)
        adc->chn_state[channel].enabled = true;
}

int main(void)
{
    hpm_rtt_adc adc = {0};

    enable_channel(&adc, 16);
    return (int)adc.adc_base;
}

Compile it with a bounds sanitizer, for example:

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

Checking channel >= 0 && channel < 16 before the first state access avoids the report. Channel -1 should be rejected the same way; on this layout it can change the preceding adc_base field without stopping at the end of a separate allocation.

Relevant log output

runtime error: member access within address ... with insufficient space
for an object of type 'adc_channel_state_t'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior

Impact

A caller that can pass a channel to rt_adc_enable(), including the adc shell command when FinSH is enabled, can index outside chn_state. The effect in this model is an out-of-range state read and, when the indexed entry looks disabled, a write. Peripheral misconfiguration or loss of availability is possible. Physical fault behavior depends on the board.

Suggested fix

A later revision of this file, commit fc64f78669b42b35981f97c23c7e952832f3b48b, removes chn_state and checks the channel before use. Trees that still contain the array should validate and map the channel before any state access, size the state for every supported mapped channel, and reject unsupported values without writing adjacent fields.

Other additional context

No response