[BUG] ISDB decoder ignores the payload length: heap-buffer-overflow in parse_statement, plus a one-byte overflow in get_text
Los mantenedores suelen responder en 1 día
Evaluación
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Aptitud para principiantes
- 55/100
Línea de trabajo
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.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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.)
- Lenguaje dominante
- C
- Estrellas
- 912
- Forks
- 598
- Merge medio
- 2 d 17 h
- PR fusionados (30 d)
- 30
Preparar el entorno
- Sin Dockerfile ni archivo de Docker Compose
- Tiene una plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de CCExtractor/ccextractor
-
Heap OOB read in parse_ad_cdp: cc_count not bounded by packet lengthPosiblemente ocupada @Nikhil-jaiswal007 la tomó hace 5 días. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
CCExtractor/ccextractor#2425 · 1 comentario ·
Los mantenedores suelen responder en 1 día
-
[BUG] Heap buffer overflow in parse_PMT with C0/C1 section lengthPosiblemente ocupada @kun-codes la tomó hace 7 días. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
CCExtractor/ccextractor#2419 ·
Los mantenedores suelen responder en 1 día
-
[BUG] Filename sequence expansion (`+`) panics on numeric values exceeding `i32::MAX`Posiblemente ocupada @muntasir-396 la tomó hace 10 días. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
CCExtractor/ccextractor#2402 ·
Los mantenedores suelen responder en 1 día
-
[BUG] WTV read_header(): unchecked len and text_len from the file crash the parser (SEGV, 4 KB reproducer)Posiblemente ocupada @pranayr710 la tomó hace 2 días. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 35/100
CCExtractor/ccextractor#2433 ·
Los mantenedores suelen responder en 1 día
-
[BUG] SIGSEGV in tlt_read_rcwt(): rcwt_loop re-checks parsebuf[6..7] after it was overwritten with caption data (608-mode RCWT)Posiblemente ocupada @VishnuSrivatsava la tomó hace 2 días. Abierto
Dificultad 3/5 1-2 días Aptitud para principiantes 72/100
CCExtractor/ccextractor#2429 ·
Los mantenedores suelen responder en 1 día
Todos los issues de CCExtractor/ccextractor
Issues similares
-
Type: Bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 76/100
apache/nuttx-apps#3942 ·
Los mantenedores suelen responder en 1 día
-
Add missing unit tests for IsAdmissionPolicyName and IsCertManagerResourceNamePosiblemente ocupada @a1h8 la tomó hoy. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
kubernetes-sigs/security-profiles-operator#3644 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 1/5 Menos de una hora Aptitud para principiantes 70/100
ly05010419/MacNativeGames#1340 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 84/100
Los mantenedores suelen responder en 1 día