[BUG] ISDB decoder ignores the payload length: heap-buffer-overflow in parse_statement, plus a one-byte overflow in get_text
I maintainer di solito rispondono entro 1 giorno
Valutazione
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Idoneità per principianti
- 55/100
Direzione di ricerca
Start in src/lib_ccx/ccx_decoders_isdb.c, following isdbsub_decode() through the parsing functions named in the report; inspect reserve_buf() and both get_text() call sites for the terminator overflow. Use the supplied transport stream generator and reproduce with an AddressSanitizer build. Done when stream lengths are bounded throughout the parser and the reported overflows no longer occur; the issue author says a patch and PR are in progress.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Summary
The ISDB caption decoder (src/lib_ccx/ccx_decoders_isdb.c) never looks at how
much data it was actually given. isdbsub_decode() takes a buf_size parameter
and does not use it, and every length below it comes straight out of the stream
and is used unchecked. Crafted .ts files make an ASan build report
heap-buffer-overflows in parse_statement() and in get_text(), or die outright.
There are two separate defects here. They share a reproducer corpus, so I am
filing them together, but they are independent.
1. Stream-supplied lengths are never bounded
isdbsub_decode() is called from general_loop.c with the demuxed PES payload
and its length:
isdbsub_decode(dec_ctx, data_node->buffer, data_node->len, dec_sub);
buf_size is then unused, and the chain below trusts the file:
isdbsub_decode()—header_end = buf + (*buf & 0x0f)can point past the
payload, and a payload shorter than three bytes is read anyway;isdb_parse_data_group()—group_size = RB16(buf)is 16 bits, so a data
group can claim 64 KB inside a 25-byte payload;parse_caption_statement_data()/parse_data_unit()— the data unit length
isRB24(buf), soparse_statement()can be asked to walk 16 MB;parse_caption_management_data()—nb_langis one byte, so the loop can
read 255 five-byte language entries out of nothing.
parse_statement() additionally passes the whole statement size to
parse_command() instead of what is left from the current position, and leaves
ret unassigned on two of its cases — indeterminate on the first pass, and a
carried-over 0 would spin buf += ret forever.
SUMMARY: AddressSanitizer: heap-buffer-overflow in parse_statement
The payload lands in ptr->buffer, a BUFSIZE (2 MB) heap allocation, so the
smaller overreads stay inside the block and quietly mix stale bytes from earlier
packets into the subtitle text; the 24-bit length is what runs off the end.
2. get_text() writes one byte past its allocation
sb_text = allocate_text_node(ls);
list_add_tail(&sb_text->list, &ctx->buffered_text);
reserve_buf(sb_text, text->used);
memcpy(sb_text->buf, text->buf, text->used);
sb_text->buf[text->used] = '\0';
reserve_buf(text, len) guarantees capacity for text->used + len, i.e. len
bytes on top of what is used. The node is fresh, so this reserves exactly
text->used bytes and the terminator goes one past the end. The allocation
rounds up to a multiple of 128, which hides it until text->used is itself an
exact multiple of 128. Two call sites in get_text() do this.
SUMMARY: AddressSanitizer: heap-buffer-overflow in get_text
Reproducing
ISDB is selected when the PMT advertises stream type 0x06 with a
data_component_descriptor (tag 0xFD) whose component_id is 0x0008
(ts_tables.c). A ~1 KB transport stream is enough; generator:
import struct
PMT_PID, SUB_PID, PROG = 0x1000, 0x1001, 1
_cc = {}
def crc32_mpeg(d):
crc = 0xFFFFFFFF
for b in d:
crc ^= b << 24
for _ in range(8):
crc = ((crc << 1) ^ 0x04C11DB7) & 0xFFFFFFFF if crc & 0x80000000 else (crc << 1) & 0xFFFFFFFF
return crc
def ts_packets(pid, payload):
out, first = b"", True
while payload:
cc = _cc.get(pid, 0); _cc[pid] = (cc + 1) & 0xF
chunk, payload = payload[:184], payload[184:]
if len(chunk) < 184:
pad = 184 - len(chunk)
af = b"\x00" if pad == 1 else bytes([pad - 1, 0x00]) + b"\xff" * (pad - 2)
body, afc = af + chunk, 0x30
else:
body, afc = chunk, 0x10
out += bytes([0x47, (0x40 if first else 0) | ((pid >> 8) & 0x1F), pid & 0xFF, afc | cc]) + body
first = False
return out
def psi(table_id, ext, body):
sec = bytes([table_id]) + struct.pack(">H", 0xB000 | (len(body) + 9)) \
+ struct.pack(">H", ext) + bytes([0xC1, 0x00, 0x00]) + body
return b"\x00" + sec + struct.pack(">I", crc32_mpeg(sec))
pat = psi(0x00, 1, struct.pack(">HH", PROG, 0xE000 | PMT_PID))
desc = bytes([0xFD, 0x02]) + struct.pack(">H", 0x0008)
b = struct.pack(">H", 0xE000 | SUB_PID) + struct.pack(">H", 0xF000)
b += bytes([0x06]) + struct.pack(">H", 0xE000 | SUB_PID) + struct.pack(">H", 0xF000 | len(desc)) + desc
pmt = psi(0x02, PROG, b)
def pes(pts, payload):
ptsb = bytes([0x21 | ((pts >> 29) & 0x0E), (pts >> 22) & 0xFF, 0x01 | ((pts >> 14) & 0xFE),
(pts >> 7) & 0xFF, 0x01 | ((pts << 1) & 0xFE)])
body = bytes([0x81, 0x80, 5]) + ptsb + payload
return b"\x00\x00\x01\xBD" + struct.pack(">H", len(body)) + body
def isdb(group_size, unit_len):
p = bytes([0x80, 0xFF, 0x00]) # synchronised PES header
p += bytes([0x04, 0x00, 0x00]) + struct.pack(">H", group_size) # data group, id 1
p += bytes([0x00]) + struct.pack(">I", unit_len)[1:] # caption statement data
p += bytes([0x1F, 0x20]) + struct.pack(">I", unit_len)[1:] # data unit, statement body
return p
payload = isdb(0xFFFF, 0xFFFFFF) # lengths far past the 17-byte payload
st = ts_packets(0x0000, pat) + ts_packets(PMT_PID, pmt)
for i in range(3): # each PES is terminated by the next one's PUSI
st += ts_packets(SUB_PID, pes(90000 + i * 9000, payload))
st += ts_packets(0x0000, pat)
open("isdb_len24.ts", "wb").write(st)
$ ccextractor --input ts isdb_len24.ts -o out.srt
*****ISDB subtitles detected
==1==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7d9b87f99878
SUMMARY: AddressSanitizer: heap-buffer-overflow in parse_statement
--input ts only because autodetect does not take to such a small synthetic
file; the decoder path is the normal one.
Four of seven files in my corpus crash on master (d7eb132) built with
-fsanitize=address,undefined: two report parse_statement, one reports
get_text, one dies in ASan's signal handler.
Suggested fix
Carry the real length down the chain and clamp each stream-supplied length to
what is left, truncating rather than erroring so data that legitimately spans
packets still decodes. Separately, ask reserve_buf() for the terminator in
both get_text() call sites.
I have a patch and will open a PR referencing this issue.
(Analysis and patch prepared with AI assistance; the reproducer and the
sanitizer runs are my own.)
- Lingua principale
- C
- Stelle
- 912
- Fork
- 598
- Merge medio
- 2g 17h
- PR unite (30g)
- 30
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Ha un modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di CCExtractor/ccextractor
-
Heap OOB read in parse_ad_cdp: cc_count not bounded by packet lengthForse già presa @Nikhil-jaiswal007 l’ha presa 4 giorni fa. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
CCExtractor/ccextractor#2425 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
[BUG] Heap buffer overflow in parse_PMT with C0/C1 section lengthForse già presa @kun-codes l’ha presa 6 giorni fa. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
CCExtractor/ccextractor#2419 ·
I maintainer di solito rispondono entro 1 giorno
-
[BUG] Filename sequence expansion (`+`) panics on numeric values exceeding `i32::MAX`Forse già presa @muntasir-396 l’ha presa 9 giorni fa. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 82/100
CCExtractor/ccextractor#2402 ·
I maintainer di solito rispondono entro 1 giorno
-
[BUG] WTV read_header(): unchecked len and text_len from the file crash the parser (SEGV, 4 KB reproducer)Forse già presa @pranayr710 l’ha presa 1 giorno fa. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 35/100
CCExtractor/ccextractor#2433 ·
I maintainer di solito rispondono entro 1 giorno
-
[BUG] SIGSEGV in tlt_read_rcwt(): rcwt_loop re-checks parsebuf[6..7] after it was overwritten with caption data (608-mode RCWT)Forse già presa @VishnuSrivatsava l’ha presa 1 giorno fa. Aperta
Difficoltà 3/5 1-2 giorni Idoneità per principianti 72/100
CCExtractor/ccextractor#2429 ·
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di CCExtractor/ccextractor
Issue simili
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
zephyrproject-rtos/zephyr#123881 ·
I maintainer di solito rispondono entro 2 giorni
-
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 65/100
ly05010419/MacNativeGames#1252 ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 70/100
I maintainer di solito rispondono entro 2 giorni
-
Component - Build
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
I maintainer di solito rispondono entro 1 giorno
-
Description of the bugApertabug
Difficoltà 2/5 1-3 ore Idoneità per principianti 68/100
HarbourMasters/Lighthouse#655 ·
I maintainer di solito rispondono entro 1 giorno