DOKIMASecurity review report

Overview

Adversarial SRT timestamp line triggers unrecovered index-out-of-range panic in astisub parser

low Fully Valid high confidence

DOK-100094-FILEBROWSER-SUBTITLE-CONVERSIO-ADVERSARIAL-SRT-TR · filebrowser · subtitle-conversion

Status

Fully Valid

Concrete, code-proven reachable panic with a specific sink (go-astisub srt.go:115 s2[0]) and a deterministic trigger input, distinct from the memory-amplification DOK-100012 (size limit) and the special-file DOK-100077 (blocking read). It supplies exactly the concrete parser-bug evidence whose absence caused DOK-100089 to be rejected as speculative, so it supersedes DOK-100089. Vulnerable path complete in source (workspace + on-disk dependency) with no unresolved precondition, hence fully_valid; severity stays low because net/http's per-connection recover bounds impact to a single aborted request plus log noise.

Repository / Component

Repository
filebrowser
Component
subtitle-conversion

Plain-English Description

A specially crafted subtitle file can crash the request that converts it. When a user requests conversion of such a file, the parser hits a malformed timestamp line and aborts that one connection. The damage is limited to the single failed request.

Description of the Underlying Issue

subtitleFileHandler calls astisub.ReadFromSRT(fd) at http/subtitle.go:50-52 for any .srt file, with no input validation and no recover(). The bundled go-astisub v0.40.0 parser treats any line containing '-->' as a time-boundary line (srt.go:60). It splits on '-->' and only checks len(s1) < 2 (srt.go:102-103); it then takes strings.Fields of the right half (srt.go:108) and indexes s2[0] with no length check (srt.go:115). A line like '00:00:00,000-->' splits into exactly two elements (passing the guard) with a canonical start timestamp that parseDurationSRT accepts, but an empty right half, so strings.Fields returns an empty slice and s2[0] is an index-out-of-range panic. The http package contains zero recover() calls, so the panic propagates to net/http's per-connection recover. This supersedes DOK-100089 (rejected as speculative) by supplying the concrete parser sink, and is distinct from the memory-amplification DOK-100012 and the special-file DOK-100077.

Potential Attack

An authenticated user with Perm.Download (to read) — and Perm.Modify to upload the file if not already present — places a file ending in .srt whose time-boundary line is '00:00:00,000-->' and issues GET /api/subtitle/<path>. subtitleHandler -> subtitleFileHandler -> astisub.ReadFromSRT reaches srt.go:115 and panics deterministically on every request to that file.

Outcomes of Potential Attack

The single HTTP request is aborted by net/http's per-connection recover, returning no VTT, plus a panic stack trace written to the log. There is no process crash, no restart loop, and no file-descriptor or goroutine leak — ReadFromSRT's fd is opened and deferred-closed in the same handler frame. Impact is bounded to one aborted connection plus log noise.

Affected Scope

GET /api/subtitle{path} for any .srt file readable in the caller's scope; sink is http/subtitle.go:52 astisub.ReadFromSRT(fd) inside subtitleFileHandler (Perm.Download-gated, authenticated).

Suggested Fix (plain english)

Wrap the subtitle parsing in error/panic handling so a malformed file returns a normal error response instead of aborting the request.

Suggested Fix (detailed)

Wrap the astisub.ReadFromSRT / ReadFromSSA calls in subtitleFileHandler with a defer/recover that converts a parse panic into an HTTP 500 (the handler already returns (int, error)); and/or validate subtitle structure before parsing; and/or track go-astisub for a release that bounds-checks s2 before indexing srt.go:115. Verify by requesting a crafted '00:00:00,000-->' .srt and confirming a clean error response with no panic stack in the log. Wrap the astisub.ReadFromSRT / ReadFromSSA calls in subtitleFileHandler in a defer/recover that converts a parse panic to HTTP 500 (the handler already returns (int, error)); and/or validate subtitle structure before parsing; and/or track go-astisub for a release that bounds-checks s2 before indexing srt.go:115.

Validation

Concrete, code-proven reachable panic with a specific sink (go-astisub srt.go:115 s2[0]) and a deterministic trigger input, distinct from the memory-amplification DOK-100012 (size limit) and the special-file DOK-100077 (blocking read). It supplies exactly the concrete parser-bug evidence whose absence caused DOK-100089 to be rejected as speculative, so it supersedes DOK-100089. Vulnerable path complete in source (workspace + on-disk dependency) with no unresolved precondition, hence fully_valid; severity stays low because net/http's per-connection recover bounds impact to a single aborted request plus log noise.

first_opinion

Full Evidence

http/subtitle.go:50-52 — for a .srt name the handler calls sub, err = astisub.ReadFromSRT(fd) with no input validation and no recover()
/home/ubuntu/go/pkg/mod/github.com/asticode/go-astisub@v0.40.0/srt.go:60 — a line is treated as a time-boundary line whenever strings.Contains(line, '-->')
srt.go:102-103 — s1 := strings.Split(line, '-->'); the only guard is len(s1) < 2
srt.go:108 — s2 := strings.Fields(s1[1]); an empty/whitespace right half yields an empty slice
srt.go:111-118 — parseDurationSRT(s1[0]) parses the (valid) start, then parseDurationSRT(s2[0]) indexes s2[0] with no length check
http/ contains zero recover() calls (grep count = 0), so the panic propagates to net/http conn.serve which recovers per-connection
Proven fact: A .srt line such as '00:00:00,000-->' splits into exactly two elements (len 2, passes the srt.go:103 guard); s1[0]='00:00:00,000' is a canonical SRT timestamp that parseDurationSRT accepts, so execution reaches srt.go:115
Proven fact: The right half is empty so strings.Fields returns an empty slice and s2[0] is an index-out-of-range panic — reachable purely from file content, deterministically, on every request to that file
Proven fact: The path is authenticated but fully within a normal user's capability: place/own a .srt in scope (Perm.Download only to read; Perm.Modify to upload) and request /api/subtitle
Proven fact: Impact is bounded to a single aborted connection: net/http's per-connection recover catches the panic (no process crash, no restart loop, no fd/goroutine leak because ReadFromSRT's fd is opened and deferred-closed in the same handler frame)
Unvalidated fact: None material to validity — the vulnerable path is complete in the workspace plus the on-disk go-astisub v0.40.0 source. The single-request bound relies on net/http's documented per-connection panic recovery, which is standard-library behavior.
http/subtitle.go:50-52 — for a .srt name the handler calls astisub.ReadFromSRT(fd) with no input validation and no recover()
go-astisub@v0.40.0/srt.go:60 — a line is a time-boundary line whenever strings.Contains(line, '-->')
go-astisub@v0.40.0/srt.go:102-103 — s1 := strings.Split(line, '-->'); only guard is len(s1) < 2
go-astisub@v0.40.0/srt.go:108 — s2 := strings.Fields(s1[1]); empty/whitespace right half yields an empty slice
go-astisub@v0.40.0/srt.go:111-118 — parseDurationSRT(s1[0]) parses the valid start, then s2[0] is indexed with no length check
http/ contains zero recover() calls (grep count = 0), so the panic reaches net/http conn.serve which recovers per-connection
Proven fact: A line such as '00:00:00,000-->' splits into exactly two elements (passes the srt.go:103 guard); s1[0] is a canonical SRT timestamp parseDurationSRT accepts, so execution reaches srt.go:115.
Proven fact: The right half is empty so strings.Fields returns an empty slice and s2[0] is an index-out-of-range panic — reachable purely from file content, deterministically, on every request to that file.
Proven fact: The path is authenticated but within a normal user's capability: place/own a .srt in scope (Perm.Download to read, Perm.Modify to upload) and request /api/subtitle.
Proven fact: Impact is bounded to a single aborted connection: net/http's per-connection recover catches the panic with no process crash, restart loop, or fd/goroutine leak.
Unvalidated fact: None material to validity — the vulnerable path is complete in the workspace plus on-disk go-astisub v0.40.0 source. The single-request bound relies on net/http's documented per-connection panic recovery (standard-library behavior).
Sink: /home/ubuntu/go/pkg/mod/github.com/asticode/go-astisub@v0.40.0/srt.go:115 parseDurationSRT(s2[0]) on empty slice from srt.go:108 strings.Fields(s1[1])
Reachable via http/subtitle.go:52 astisub.ReadFromSRT(fd) with no recover()
Trigger input: an .srt line like '00:00:00,000 -->' (valid start, separator, nothing after)
parseDuration TrimSpaces internally (subtitles.go:1097/1115/1131) so the start half parses and execution reaches the panic
grep recover() in /workspace/filebrowser/http -> zero hits; net/http per-connection recover bounds impact to a single connection abort (no process crash, no fd leak)