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

Offen Anfängerfreundlich
  • Stimmt etwas nicht?
#11,948 1 Kommentar 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Maintainer antworten meist innerhalb von 1 Tag

@usualtreee arbeitet bereits daran.

Seit 31.10.2026.

  • #11962 von @usualtreee — offen

Bewertung

Schwierigkeit
2/5
Geschätzter Aufwand
1-3 Stunden
Anfängerfreundlichkeit
72/100
Issue-Typ
Bug
Klarheit
Klar beschrieben
Aktivitätsstatus
Aktiv
Tech-Stack
c
Bereich
embedded-iot, security

Rechercherichtung

Beginne mit den AB32-ADC-Callbacks in bsp/bluetrum/libraries/hal_drivers/drv_adc.c und den Callback-Definitionen in components/drivers/include/drivers/adc.h. Verfolge, wie Kanalwerte die HAL-Konvertierung und den SADCDAT-Registerzeiger erreichen; prüfe den Bereich mit sechzehn Registern und das Konvertierungsergebnis, bevor ein Messwert zurückgegeben wird. Fertig ist die Änderung, wenn nicht unterstützte Kanäle, einschließlich negativer Konstanten für interne Kanäle, abgewiesen werden und gültige Kanäle denselben geprüften Wert für die Konvertierung und das Auslesen verwenden.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Beschreibung

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

Vorherrschende Sprache
C
Sterne
12.3k
Forks
5.5k
Ø Merge
4 T. 9 Std.
Gemergte PRs (30 T.)
38

Entwicklungsumgebung

In Codespaces öffnen

Startet den Dev-Container des Projekts im Browser, mit Ihrem eigenen GitHub-Konto.

Erste Schritte

  1. Lesen Sie das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreiben Sie ins Issue, dass Sie es übernehmen — das erspart doppelte Arbeit.
  3. Forken Sie das Repository und arbeiten Sie in einem Branch.
  4. Öffnen Sie einen Pull Request, der die Issue-Nummer nennt.

Mehr aus RT-Thread/rt-thread

Alle Issues in RT-Thread/rt-thread

Ähnliche Issues

Weitere Issues zu C

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.