Fix SSRF in /video proxy and API JSON injection

Validate the target host on the /video media route with
isTwitterUrl() (mirroring /pic) and reject non-http(s) schemes,
and stop the media proxy following redirects off the validated
host. JSON-escape user-controlled GraphQL cursors and build id
variables with packedjson so untrusted input can't break out of
the query. Warn on startup when the insecure default hmacKey is
in use.

Fixes #1411

nitter.example.conf

@@ -20,7 +20,7 @@ redisMaxConnections = 30
 # you receive tons of requests per second
 
 [Config]
-hmacKey = "secretkey"        # random key for cryptographic signing of video urls
+hmacKey = "secretkey"        # CHANGE THIS to a unique random value (e.g. `openssl rand -hex 32`); signs media urls
 base64Media = false          # use base64 encoding for proxied media urls
 enableRSS = true             # master switch, set to false to disable all RSS feeds
 enableRSSUserTweets = true   # /@user/rss

src/api.nim

@@ -18,6 +18,11 @@ proc apiReq(endpoint, variables: string; fieldToggles = ""; skipTid = false): Ap
   let url = apiUrl(endpoint, variables, fieldToggles, skipTid)
   return ApiReq(cookie: url, oauth: url)
 
+proc cursorParam(after: string): string =
+  ## JSON-escape the user-supplied cursor so it cannot break out of the GraphQL
+  ## variables object (same input-validation class as the #1411 media SSRF).
+  if after.len > 0: "\"cursor\":" & $(%after) & "," else: ""
+
 proc mediaUrl(id, cursor: string; count=20): ApiReq =
   result = ApiReq(
     cookie: apiUrl(graphUserMedia, userMediaVars % [id, cursor, $count]),
@@ -39,10 +44,10 @@ proc tweetDetailUrl(id: string; cursor: string): ApiReq =
   # )
 
 proc userUrl(username: string): ApiReq =
-  let cookieVars = """{"screen_name":"$1","withGrokTranslatedBio":false}""" % username
+  let cookieVars = $(%*{"screen_name": username, "withGrokTranslatedBio": false})
   result = ApiReq(
     cookie: apiUrl(graphUser, cookieVars, tweetDetailFieldToggles),
-    oauth: apiUrl(graphUserV2, """{"screen_name": "$1"}""" % username)
+    oauth: apiUrl(graphUserV2, $(%*{"screen_name": username}))
   )
 
 proc getGraphUser*(username: string): Future[User] {.async.} =
@@ -60,7 +65,7 @@ proc getGraphUserById*(id: string): Future[User] {.async.} =
 proc getAboutAccount*(username: string): Future[AccountInfo] {.async.} =
   if username.len == 0: return
   let
-    url = apiReq(graphAboutAccount, """{"screenName":"$1"}""" % username)
+    url = apiReq(graphAboutAccount, $(%*{"screenName": username}))
     js = await fetch(url)
   result = parseAboutAccount(js)
 
@@ -71,7 +76,7 @@ proc restReq(endpoint: string; params: seq[(string, string)] = @[]): ApiReq =
 proc getBroadcastInfo*(id: string): Future[Broadcast] {.async.} =
   if id.len == 0: return
   let
-    req = apiReq(graphBroadcast, """{"id":"$1"}""" % id)
+    req = apiReq(graphBroadcast, $(%*{"id": id}))
     js = await fetch(req)
   result = parseBroadcastInfo(js)
 
@@ -86,7 +91,7 @@ proc fetchBroadcastStream*(mediaKey: string): Future[string] {.async.} =
 proc getGraphUserTweets*(id: string; kind: TimelineKind; after=""): Future[Profile] {.async.} =
   if id.len == 0: return
   let
-    cursor = if after.len > 0: "\"cursor\":\"$1\"," % after else: ""
+    cursor = cursorParam(after)
     url = case kind
       of TimelineKind.tweets: userTweetsUrl(id, cursor)
       of TimelineKind.replies: userTweetsAndRepliesUrl(id, cursor)
@@ -97,14 +102,14 @@ proc getGraphUserTweets*(id: string; kind: TimelineKind; after=""): Future[Profi
 proc getGraphCommunity*(id: string): Future[Community] {.async.} =
   if id.len == 0: return
   let
-    url = apiReq(graphCommunity, communityVars % id)
+    url = apiReq(graphCommunity, $(%*{"communityId": id}))
     js = await fetch(url)
   result = parseGraphCommunity(js)
 
 proc getGraphCommunityTweets*(id: string; rankingMode: string; after=""): Future[Timeline] {.async.} =
   if id.len == 0: return
   let
-    cursor = if after.len > 0: "\"cursor\":\"$1\"," % after else: ""
+    cursor = cursorParam(after)
     url = apiReq(graphCommunityTweets, communityTweetsVars % [id, cursor, rankingMode])
     js = await fetch(url)
   result = parseGraphCommunityTimeline(js, after)
@@ -112,7 +117,7 @@ proc getGraphCommunityTweets*(id: string; rankingMode: string; after=""): Future
 proc getGraphCommunityMedia*(id: string; after=""): Future[Timeline] {.async.} =
   if id.len == 0: return
   let
-    cursor = if after.len > 0: "\"cursor\":\"$1\"," % after else: ""
+    cursor = cursorParam(after)
     url = apiReq(graphCommunityMedia, communityMediaVars % [id, cursor])
     js = await fetch(url)
   result = parseGraphCommunityTimeline(js, after)
@@ -124,7 +129,7 @@ proc communitySliceReq(endpoint, variables: string): ApiReq =
 proc getGraphCommunityMembers*(id: string; after=""): Future[Result[User]] {.async.} =
   if id.len == 0: return
   let
-    cursor = if after.len > 0: "\"$1\"" % after else: "null"
+    cursor = if after.len > 0: $(%after) else: "null"
     url = communitySliceReq(graphCommunityMembers, communityMembersVars % [id, cursor])
     js = await fetch(url)
   result = parseGraphCommunityMembers(js, after)
@@ -140,7 +145,7 @@ proc getGraphCommunityHashtags*(id, hashtag: string; after=""): Future[Timeline]
   if id.len == 0 or hashtag.len == 0: return
   let
     safeTag = multiReplace(hashtag, ("\"", ""), ("\\", ""))
-    cursor = if after.len > 0: "\"cursor\":\"$1\"," % after else: ""
+    cursor = cursorParam(after)
     url = apiReq(graphCommunityHashtags, communityHashtagsVars % [id, cursor, safeTag])
     js = await fetch(url)
   result = parseGraphCommunityTimeline(js, after)
@@ -148,7 +153,7 @@ proc getGraphCommunityHashtags*(id, hashtag: string; after=""): Future[Timeline]
 proc getGraphListTweets*(id: string; after=""): Future[Timeline] {.async.} =
   if id.len == 0: return
   let
-    cursor = if after.len > 0: "\"cursor\":\"$1\"," % after else: ""
+    cursor = cursorParam(after)
     url = apiReq(graphListTweets, restIdVars % [id, cursor, "20"])
     js = await fetch(url)
   result = parseGraphTimeline(js, after).tweets
@@ -162,7 +167,7 @@ proc getGraphListBySlug*(name, list: string): Future[List] {.async.} =
 
 proc getGraphList*(id: string): Future[List] {.async.} =
   let 
-    url = apiReq(graphListById, """{"listId": "$1"}""" % id)
+    url = apiReq(graphListById, $(%*{"listId": id}))
     js = await fetch(url)
   result = parseGraphList(js)
 
@@ -186,14 +191,14 @@ proc getGraphListMembers*(list: List; after=""): Future[Result[User]] {.async.}
 proc getGraphTweetResult*(id: string): Future[Tweet] {.async.} =
   if id.len == 0: return
   let
-    url = apiReq(graphTweetResult, """{"rest_id": "$1"}""" % id)
+    url = apiReq(graphTweetResult, $(%*{"rest_id": id}))
     js = await fetch(url)
   result = parseGraphTweetResult(js)
 
 proc getGraphTweet(id: string; after=""): Future[Conversation] {.async.} =
   if id.len == 0: return
   let
-    cursor = if after.len > 0: "\"cursor\":\"$1\"," % after else: ""
+    cursor = cursorParam(after)
     js = await fetch(tweetDetailUrl(id, cursor))
   result = parseGraphConversation(js, id)
 

src/consts.nim

@@ -161,8 +161,6 @@ const
 
   articleFieldToggles* = """{"withArticleRichContentState":true,"withArticlePlainText":false,"withArticleSummaryText":true,"withArticleVoiceOver":true}"""
 
-  communityVars* = """{"communityId":"$1"}"""
-
   communityTweetsVars* = """{
   "communityId": "$1", $2
   "count": 20,

src/nitter.nim

@@ -34,6 +34,10 @@ stdout.flushFile
 updateDefaultPrefs(fullCfg)
 setCacheTimes(cfg)
 setHmacKey(cfg.hmacKey)
+if cfg.hmacKey.len == 0 or cfg.hmacKey == "secretkey":
+  stderr.write "WARNING: insecure default 'hmacKey' in nitter.conf; " &
+    "set a unique random value to stop media URL signatures being forgeable.\n"
+  stderr.flushFile
 setProxyEncoding(cfg.base64Media)
 setMaxHttpConns(cfg.httpMaxConns)
 setHttpProxy(cfg.proxy, cfg.proxyAuth)

src/routes/media.nim

@@ -15,7 +15,9 @@ const
   maxAge* = "max-age=604800"
 
 proc safeFetch*(url: string): Future[string] {.async.} =
-  let client = newAsyncHttpClient()
+  # maxRedirects=0: the caller already validated the host, so never follow a
+  # redirect off the allowlisted host (would re-open the #1411 SSRF).
+  let client = newAsyncHttpClient(maxRedirects = 0)
   try: result = await client.getContent(url)
   except: discard
   finally: client.close()
@@ -32,7 +34,7 @@ proc proxyMedia*(req: jester.Request; url: string): Future[HttpCode] {.async.} =
   result = Http200
   let
     request = req.getNativeReq()
-    client = newAsyncHttpClient()
+    client = newAsyncHttpClient(maxRedirects = 0)
 
   try:
     let res = await client.get(url)
@@ -122,7 +124,7 @@ proc createMediaRouter*(cfg: Config) =
 
     get re"^\/video\/(enc)?\/?(.+)\/(.+)$":
       let url = decoded(request, 2)
-      cond "http" in url
+      cond isTwitterUrl(url)
 
       if getHmac(url) != request.matches[1]:
         resp Http403, showError("Failed to verify signature", cfg)

src/utils.nim

@@ -57,8 +57,8 @@ proc filterParams*(params: Table): seq[(string, string)] =
       result.add p
 
 proc isTwitterUrl*(uri: Uri): bool =
-  uri.hostname in twitterDomains or
-    uri.hostname.endsWith(".video.pscp.tv")
+  uri.scheme in ["http", "https"] and
+    (uri.hostname in twitterDomains or uri.hostname.endsWith(".video.pscp.tv"))
 
 proc isTwitterUrl*(url: string): bool =
   isTwitterUrl(parseUri(url))

tests/test_ssrf_1411.nim

@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: AGPL-3.0-only
+# Reproduction + regression test for issue #1411:
+# SSRF via /video proxy with default HMAC key and missing host validation.
+import std/[unittest, uri]
+import ".."/src/utils
+
+suite "issue #1411 SSRF via /video proxy":
+  setup:
+    # The default key shipped in nitter.example.conf / config.nim.
+    setHmacKey("secretkey")
+
+  test "HMAC for arbitrary SSRF URLs is forgeable with the default key":
+    # These signatures were independently computed (Python hmac-sha256, uppercase
+    # hex, first 13 chars) and observed live in the issue report.
+    check getHmac("http://172.17.0.1:19999/secret_data.m3u8") == "BBD19ACC6C012"
+    check getHmac("http://172.17.0.1:19999/secret_data.mp4")  == "0780F00DDF3E7"
+
+  test "isTwitterUrl rejects SSRF targets (the guard /video is missing)":
+    # Internal / metadata hosts an attacker would target.
+    check isTwitterUrl(parseUri("http://172.17.0.1:19999/secret_data.m3u8")) == false
+    check isTwitterUrl(parseUri("http://169.254.169.254/latest/meta-data/x.m3u8")) == false
+    check isTwitterUrl(parseUri("http://localhost/x.mp4")) == false
+    check isTwitterUrl(parseUri("http://[::1]/x.mp4")) == false
+
+  test "isTwitterUrl rejects userinfo / look-alike host bypass attempts":
+    check isTwitterUrl(parseUri("http://video.twimg.com@169.254.169.254/x.mp4")) == false
+    check isTwitterUrl(parseUri("http://video.twimg.com.evil.com/x.mp4")) == false
+    check isTwitterUrl(parseUri("http://evilvideo.twimg.com.attacker/x.mp4")) == false
+
+  test "isTwitterUrl rejects non-http schemes even on a Twitter host":
+    check isTwitterUrl(parseUri("gopher://video.twimg.com/x.mp4")) == false
+    check isTwitterUrl(parseUri("file:///etc/passwd")) == false
+    check isTwitterUrl(parseUri("ftp://video.twimg.com/x.mp4")) == false
+
+  test "isTwitterUrl still allows legitimate Twitter video hosts":
+    check isTwitterUrl(parseUri("https://video.twimg.com/ext_tw_video/1/pu/pl/x.m3u8")) == true
+    check isTwitterUrl(parseUri("https://video.twimg.com/amplify_video/1/vid/x.mp4")) == true
+    check isTwitterUrl(parseUri("https://prod-fastly-us-east-1.video.pscp.tv/x.m3u8")) == true