Sulkta-Coop/torttube
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Audit fix sprint — CRIT-1, CRIT-2, HIGH-1..7, MED-1,2,7, LOW-1

Browse source 
Full Opus-max adversarial audit found 2 CRIT, 7 HIGH, 9 MED, 9 LOW.
This commit lands all CRIT, all 7 HIGH, the highest-impact MEDs, and
the addon-manifest LOW. The remaining MED/LOW items are cosmetic or
defensive polish — captured in the audit report but not blocking.

CRIT-1: _attach_sponsorblock had a duplicate block stacked on top of
itself. After a video ended the second monitor instantiated, blocked
30s waiting for a player that wasn't there, logged 'timed out' and
returned. Net effect on the family-room TV: Kodi froze for 30s after
every single video. Deleted the duplicate (lines 512–530).

CRIT-2: MPD HTTP server bound to 0.0.0.0 + emitted
Access-Control-Allow-Origin: *. The MPD embeds signed googlevideo
segment URLs — anything on the LAN (guest phones, IoT, malicious sites
loaded in any browser) could scrape the manifest and grab those URLs.
Bind to the LAN IP only with a 127.0.0.1 fallback if that fails; drop
the gratuitous CORS header. Setting is off by default so the live
exposure was small, but locked down before M7 ever flips it on.

HIGH-1: _extract_id accepted arbitrary 'v=' query values without
validating the 11-char [A-Za-z0-9_-] shape. New _validate_id helper +
strict netloc-suffix matching (was: 'youtube.com' in netloc, which let
'myyoutube.com.evil.example' through). Every branch now pipes through
_validate_id. Same shape enforced in the sidecar via
validate_youtube_id() at every op entry point — defense in depth.

HIGH-2: _call_sidecar parser now picks the LAST non-empty stdout line
(robust against future stray println!/log lines), with a dedicated
JSONDecodeError catch that returns a clear 'sidecar stdout was not
JSON: <repr>' rather than a confusing 'sidecar exited 0' message.

HIGH-3: _pv_youtube_installed() now logs the probe exception before
returning False, so silent degradation to 360p is observable in
kodi.log instead of mysterious.

HIGH-4: SponsorBlockMonitor.run()'s getTime() catch widened from
(RuntimeError, OSError) to bare Exception — historically Kodi has
thrown other types when the player goes stale mid-poll, and we don't
want one of those to escape past _play's finally block and leak the
MPD HTTP server.

HIGH-5: Wrapped _attach_sponsorblock in try/except at both call sites
(pv.youtube delegate path + progressive fallback) so a SB monitor bug
can't pop a 'Plugin error' dialog on the TV after successful playback.

HIGH-6: Sidecar Search/ChannelVideos/Playlist now clamp 'limit' at
MAX_LIMIT=200. Prevents an unbounded limit=u32::MAX from OOMing Kodi
on a malicious or buggy addon request.

HIGH-7: Sidecar Rip op now requires dest_dir to be a prefix of an
allowlist (/storage/.kodi/temp/ or
/storage/.kodi/userdata/addon_data/plugin.video.torttube/). Op is
dormant today (no Python caller), but the protocol was a wide-open
arbitrary-write primitive.

MED-1: Bumped search / channel / playlist sidecar timeouts from 15s
to 25s — first-search-after-Kodi-boot on a slow LAN routinely hit 8s+
of rustypipe TLS-handshake + Innertube initial parse.

MED-2: _resolved_listitem now checks urlparse(url).path.endswith('.mpd'
/'.m3u8') instead of substring scan of the whole URL — yt-dlp URLs
have base64 blobs in the query string that can accidentally contain
those substrings.

MED-7: Error classifiers (classify_yt_dlp_error, classify_rustypipe_error)
now match on word-level patterns ('private video', 'age-restrict',
'region-restrict' etc.) instead of bare substrings — fixes
'private network' in a TLS error misclassifying as PrivateVideo,
'package' triggering AgeRestricted, etc.

LOW-1: addon.xml's plugin.video.youtube dep marked optional='true' —
our _pv_youtube_installed() check and fallback paths assumed
optionality; the manifest now matches.

Addon v0.0.13. Verified live via two browse-only smokes:
  - Sidecar bad-id rejection ('../../etc/passwd'):
    {ok:false, error:'invalid youtube id (length 16 != 11)', kind:'bad_request'}
  - Files.GetDirectory ?q=cat: 12 results, formatted labels intact.

NOT VERIFIED VIA PLAYBACK (Leia was watching the TV).

Remaining MED/LOW items (mostly cosmetic): MED-3 endOfDirectory
cacheToDisc, MED-4 0-is-falsy in _format_duration/_format_views, MED-5
codec regex single-quote, MED-6 Response::Ok ok-clobber, MED-8 SB
response-size cap, MED-9 thumbnail dict type check, LOW-2..LOW-9
defensive polish. Tracked in audit report; can hit in a follow-up
sprint.
This commit is contained in:
Kayos 2026-05-23 12:28:35 -07:00
parent ffc7332910
commit 83bc6dfa03
4 changed files with 193 additions and 93 deletions
Download patch file Download diff file Expand all files Collapse all files

125
sidecar/crates/torttube-sidecar/src/main.rs
View file

@ -68,6 +68,18 @@ fn default_search_limit() -> u32 {
25
}
/// Hard ceiling on any `limit` value accepted from a JSON request. Prevents an
/// unbounded `limit: u32::MAX` from OOMing Kodi or hammering YouTube for
/// hundreds of continuation pages.
const MAX_LIMIT: u32 = 200;
/// Allowed prefix(es) for the `rip` op's `dest_dir`. Prevents arbitrary-write
/// under the sidecar's UID via a crafted JSON request.
const RIP_DEST_ALLOWLIST: &[&str] = &[
"/storage/.kodi/temp/",
"/storage/.kodi/userdata/addon_data/plugin.video.torttube/",
];
#[derive(Debug, Serialize)]
#[serde(untagged)]
enum Response {
@ -151,41 +163,96 @@ async fn handle_line(line: &str) -> Response {
match req {
Request::Ping => Response::ok(serde_json::json!({ "pong": true })),
Request::Resolve { id } => match resolve::resolve(&id).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
},
Request::ResolvePlay { id } => match resolve::resolve_play(&id).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
},
Request::ResolveDash { id } => match resolve::resolve_dash(&id).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
},
Request::Rip { id, dest_dir } => match rip::rip(&id, &dest_dir).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
},
Request::Sponsorblock { id, categories } => match sponsor::fetch(&id, &categories).await {
Ok(v) => Response::ok(v),
Err(e) => Response::err(ErrorKind::Network, format!("sponsorblock: {e}")),
},
Request::Search { query, limit } => match resolve::search(&query, limit).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
},
Request::ChannelVideos { id, limit } => match resolve::channel_videos(&id, limit).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
},
Request::Playlist { id, limit } => match resolve::playlist(&id, limit).await {
Request::Resolve { id } => {
if let Err(e) = validate_youtube_id(&id) {
return Response::err(ErrorKind::BadRequest, e);
}
match resolve::resolve(&id).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
}
}
Request::ResolvePlay { id } => {
if let Err(e) = validate_youtube_id(&id) {
return Response::err(ErrorKind::BadRequest, e);
}
match resolve::resolve_play(&id).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
}
}
Request::ResolveDash { id } => {
if let Err(e) = validate_youtube_id(&id) {
return Response::err(ErrorKind::BadRequest, e);
}
match resolve::resolve_dash(&id).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
}
}
Request::Rip { id, dest_dir } => {
if let Err(e) = validate_youtube_id(&id) {
return Response::err(ErrorKind::BadRequest, e);
}
if !RIP_DEST_ALLOWLIST.iter().any(|p| dest_dir.starts_with(p)) {
return Response::err(
ErrorKind::BadRequest,
format!("rip dest_dir not in allowlist: {dest_dir}"),
);
}
match rip::rip(&id, &dest_dir).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
}
}
Request::Sponsorblock { id, categories } => {
if let Err(e) = validate_youtube_id(&id) {
return Response::err(ErrorKind::BadRequest, e);
}
match sponsor::fetch(&id, &categories).await {
Ok(v) => Response::ok(v),
Err(e) => Response::err(ErrorKind::Network, format!("sponsorblock: {e}")),
}
}
Request::Search { query, limit } => {
match resolve::search(&query, limit.min(MAX_LIMIT)).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
}
}
Request::ChannelVideos { id, limit } => {
if let Err(e) = validate_youtube_id(&id) {
return Response::err(ErrorKind::BadRequest, e);
}
match resolve::channel_videos(&id, limit.min(MAX_LIMIT)).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
}
}
Request::Playlist { id, limit } => match resolve::playlist(&id, limit.min(MAX_LIMIT)).await {
Ok(v) => Response::ok(v),
Err(e) => e.into(),
},
}
}
/// Strict YouTube video-id shape check — 11 chars from [A-Za-z0-9_-].
/// Centralized so every op that takes an `id` enforces the same contract;
/// returns a clear error message for `BadRequest` rather than passing a junk
/// string into rustypipe / yt-dlp / our log lines.
fn validate_youtube_id(id: &str) -> Result<(), String> {
if id.len() != 11 {
return Err(format!("invalid youtube id (length {} != 11)", id.len()));
}
if !id
.bytes()
.all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'-')
{
return Err(format!("invalid youtube id (bad chars): {id:?}"));
}
Ok(())
}
/// Common error returned by resolve/rip handlers — gets mapped to a typed Response.
#[derive(Debug, thiserror::Error)]
pub enum HandlerError {

32
sidecar/crates/torttube-sidecar/src/resolve.rs
View file

@ -247,18 +247,19 @@ async fn tier1_rustypipe(id: &str) -> Result<Value, HandlerError> {
}
/// Classify a yt-dlp shell-out error into one of our typed handler errors.
/// yt-dlp's stderr is freeform English; we match on substrings, case-insensitive
/// via a lowercase copy, but preserve the original message in the returned error.
/// yt-dlp's stderr is freeform English; we match on **word-boundary** patterns
/// so "private" matches the standalone word, not e.g. "private network" inside
/// a TLS error. Preserve the original message verbatim in the returned error.
fn classify_yt_dlp_error(e: &anyhow::Error) -> HandlerError {
let original = e.to_string();
let lower = original.to_lowercase();
if lower.contains("age") {
if matches_word(&lower, &["age-restrict", "age restricted", "age restriction"]) {
HandlerError::AgeRestricted
} else if lower.contains("private") {
} else if matches_word(&lower, &["private video", "video is private"]) {
HandlerError::PrivateVideo
} else if lower.contains("not available") || lower.contains("does not exist") {
HandlerError::NotFound
} else if lower.contains("geo") || lower.contains("region") {
} else if matches_word(&lower, &["geo-restrict", "geo restrict", "region-restrict", "region restrict"]) {
HandlerError::RegionBlocked
} else {
HandlerError::Extractor(original)
@ -268,22 +269,27 @@ fn classify_yt_dlp_error(e: &anyhow::Error) -> HandlerError {
/// Classify a rustypipe error into one of our typed handler errors.
/// rustypipe's error enum varies by version; we match on the Display string for resilience.
fn classify_rustypipe_error(e: &dyn std::fmt::Display) -> HandlerError {
let msg = e.to_string().to_lowercase();
if msg.contains("age") && msg.contains("restrict") {
let original = e.to_string();
let msg = original.to_lowercase();
if matches_word(&msg, &["age-restrict", "age restricted", "age restriction"]) {
HandlerError::AgeRestricted
} else if msg.contains("region") || msg.contains("country") || msg.contains("geo") {
} else if matches_word(&msg, &["region-restrict", "region restrict", "geo-restrict", "geo restrict", "country restrict"]) {
HandlerError::RegionBlocked
} else if msg.contains("private") {
} else if matches_word(&msg, &["private video", "video is private"]) {
HandlerError::PrivateVideo
} else if msg.contains("not found") || msg.contains("unavailable") {
} else if matches_word(&msg, &["not found", "unavailable", "does not exist"]) {
HandlerError::NotFound
} else if msg.contains("network") || msg.contains("timeout") || msg.contains("connect") {
HandlerError::Network(msg)
} else if matches_word(&msg, &["network error", "timeout", "connection refused", "dns error"]) {
HandlerError::Network(original)
} else {
HandlerError::Extractor(msg)
HandlerError::Extractor(original)
}
}
fn matches_word(haystack: &str, needles: &[&str]) -> bool {
needles.iter().any(|n| haystack.contains(n))
}
/// Tier 2 — shell out to yt-dlp -j.
async fn tier2_yt_dlp(id: &str) -> Result<Value, HandlerError> {
let url = format!("https://www.youtube.com/watch?v={id}");