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

Aberta Para iniciantes
  • Algo errado?
#11,951 1 comentário 0 reações 0 responsáveis Ver no GitHub

Mantenedores costumam responder em até 2 dias

Ninguém assumiu esta issue ainda.

Avaliação

Dificuldade
2/5
Tempo estimado
1-3 horas
Facilidade para iniciantes
67/100
Tipo de issue
Bug
Clareza
Claramente especificada
Status de atividade
Ativa
Stack de tecnologia
c
Domínio
backend, embedded-iot

Direção de pesquisa

Leia bsp/hpmicro/libraries/drivers/drv_adc.c nas proximidades de hpm_adc_enabled() e init_channel_config() e, em seguida, verifique como components/drivers/misc/adc.c passa os canais para o driver. Garanta que canais inválidos sejam rejeitados antes de qualquer acesso a chn_state, inclusive valores negativos, e execute a reprodução com bounds-sanitizer descrita na issue. O trabalho estará concluído quando canais não suportados retornarem com segurança, sem ler nem gravar fora do array de estado.

Escrita pelo modelo de indexação a partir do texto da issue.

Descrição

bug
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

Linguagem predominante
C
Estrelas
12.3k
Forks
5.5k
Merge médio
7d 8h
PRs com merge (30d)
65

Preparar o ambiente

Abrir no Codespaces

Inicia o contêiner de desenvolvimento do projeto no navegador, com a sua própria conta do GitHub.

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Mais de RT-Thread/rt-thread

Todas as issues de RT-Thread/rt-thread

Issues semelhantes

Mais issues de C

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.