Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

BLE handlers use length fields from the packet without checking how much data arrived

Aperta
  • Qualcosa non va?
#2,485 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

@DustinHab ci sta già lavorando.

Dal 12/9/2026.

  • #2486 di @DustinHab — aperta

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
56/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Attiva
Stack tecnologico
cpp
Ambito
embedded-iot, security

Direzione di ricerca

Inizia dagli handler in FSService.cpp, DfuService.cpp e SimpleWeatherService.cpp, confrontandoli con AlertNotificationService::OnAlert e i relativi controlli OS_MBUF_PKTLEN. Esamina i percorsi interessati DELETE, MKDIR, LISTDIR, MOVE, init, READ, WRITE e di parsing del meteo, quindi esegui i controlli firmware pertinenti e cppcheck. Il lavoro è completato quando i pacchetti brevi e di dimensioni malevole vengono rifiutati in sicurezza, senza allocazioni sullo stack di lunghezza variabile, accessi fuori dai limiti o il fallthrough di MOVE.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

I searched open and closed issues and PRs and did not find these.

Several BLE write handlers cast a header struct directly onto om->om_data and then use length fields out of that header without checking how much data actually arrived. AlertNotificationService::OnAlert does it properly with OS_MBUF_PKTLEN and os_mbuf_copydata, so this looks like an oversight in the other services rather than a deliberate convention.

FSService and DfuService sit behind GetDfuAndFsMode(), which is off by default, so reaching those needs the user to have enabled firmware updates or file access first. SimpleWeatherService has no such check. None of them require bonding.

Variable length arrays sized by the peer

FSService.cpp, in DELETE, MKDIR, LISTDIR and MOVE:

uint16_t plen = header->pathlen;
char path[plen + 1] = {0};

DfuService.cpp, in the init packet parser:

uint16_t softdeviceArrayLength = om->om_data[8] + (om->om_data[9] << 8);
uint16_t sd[softdeviceArrayLength];

Both lengths come straight from the packet, so a peer can ask for 64 KB or 128 KB of stack on a part that has 64 KB of RAM in total.

Write past the end of the received buffer

FSService.cpp:304, MOVE:

uint16_t plen = header->OldPathLength;
header->pathstr[plen] = 0;

The offset comes from the packet and nothing limits it to the bytes that were received.

Off by one on filepath

FSService.cpp:98 and :165, READ and WRITE:

static constexpr uint16_t maxpathlen = 256;
char filepath[maxpathlen];
...
if (plen > maxpathlen) { ... }
memcpy(filepath, header->pathstr, plen);
filepath[plen] = 0;

plen == 256 passes the check, so filepath[256] writes into the next member, int fileSize. The comment on that line reads "counts for null term", so >= looks like the intent.

cppcheck reports this one as arrayIndexOutOfBoundsCond.

Reads past the end in the weather parser

SimpleWeatherService.cpp reads up to dataBuffer[52] in CreateCurrentWeather and up to dataBuffer[35] in CreateForecast, and copies 32 bytes from offset 16 into the location string, none of it checked against the packet length. On a short write the city name shown on the watch face contains whatever was next in memory. nbDays is clamped, so the forecast loop itself stays in bounds.

Unrelated, same file

The MOVE case in FSService::OnFSServiceRequested has no break before default:. Harmless today because default only breaks, but a case added in between would run silently.

Preferred solution

Check OS_MBUF_PKTLEN once at the top of each handler, reject anything shorter than the header it is about to read, and clamp path lengths against both the fixed buffer and the remaining packet bytes. Replace the variable length arrays with fixed buffers of maxpathlen. Change plen > maxpathlen to >=.

Found by reading the code. I have not written a proof of concept.

I can send a PR for this if you want it fixed along those lines.

Version

main, 6c119eb5

Lingua principale
C
Stelle
3.4k
Fork
1.1k
Merge medio
23h 18m
PR unite (30g)
1

Preparare l'ambiente

Apri in Codespaces

Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di InfiniTimeOrg/InfiniTime

Tutte le issue di InfiniTimeOrg/InfiniTime

Issue simili

Altre issue su C

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.