Permalink
Browse files

Start adding credentials to media embed iframe URLs.

By knowing the ID36 of a link, it is possible to see its media embed
because the embed request is served off-domain and as a result can't
verify the user's cookie.  To fix this, we add an authentication code to
the iframe URL for media embeds and require its presence for all embeds
in private subreddits.

This starts appending credentials to private subreddit embeds such that
when the latter half of the fix is deployed all apps are already
generating appropriate embed URLs.

This is part of a fix for an information disclosure vulnerability
reported by Jordan Milne (/u/largenocream).
  • Loading branch information...
1 parent 45c663e commit a6063c58434d5d4ff3717a05b989220a51fde412 @spladug spladug committed Feb 15, 2014
Showing with 21 additions and 4 deletions.
  1. +2 −0 r2/example.ini
  2. +2 −1 r2/r2/config/routing.py
  3. +16 −2 r2/r2/lib/pages/pages.py
  4. +1 −1 r2/r2/templates/mediaembed.html
View
@@ -19,6 +19,8 @@ ADMINSECRET = YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXowMTIzNDU2Nzg5
websocket = YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXowMTIzNDU2Nzg5
# secret for validating HTTP_TRUE_CLIENT_IP_HASH as sent by the CDN
true_ip =
+# secret for authenticating private media embeds
+media_embed = YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXowMTIzNDU2Nzg5
[DEFAULT]
############################################ SITE-SPECIFIC OPTIONS
View
@@ -375,7 +375,8 @@ def make_map():
mc('/captcha/:iden', controller='captcha', action='captchaimg')
- mc('/mediaembed/:link', controller="mediaembed", action="mediaembed")
+ mc('/mediaembed/:link/:credentials',
+ controller="mediaembed", action="mediaembed", credentials=None)
mc('/code', controller='redirect', action='redirect',
dest='http://github.com/reddit/')
View
@@ -92,6 +92,8 @@
from babel.dates import format_date
from collections import defaultdict
import csv
+import hmac
+import hashlib
import cStringIO
import pytz
import sys, random, datetime, calendar, simplejson, re, time
@@ -3796,11 +3798,14 @@ def make_link_child(item):
media_embed = None
if media_embed:
+ should_authenticate = (item.subreddit.type == "private")
media_embed = MediaEmbed(media_domain = g.media_domain,
height = media_embed.height + 10,
width = media_embed.width + 10,
scrolling = media_embed.scrolling,
- id36 = item._id36)
+ id36 = item._id36,
+ authenticated=should_authenticate,
+ )
else:
g.log.debug("media_object without media_embed %s" % item)
@@ -3836,7 +3841,16 @@ def content(self):
class MediaEmbed(Templated):
"""The actual rendered iframe for a media child"""
- pass
+
+ def __init__(self, *args, **kwargs):
+ authenticated = kwargs.pop("authenticated", False)
+ if authenticated:
+ mac = hmac.new(g.secrets["media_embed"], kwargs["id36"],
+ hashlib.sha1)
+ self.credentials = "/" + mac.hexdigest()
+ else:
+ self.credentials = ""
+ Templated.__init__(self, *args, **kwargs)
class SelfTextChild(LinkChild):
@@ -23,7 +23,7 @@
<%!
from r2.lib.utils import randstr
%>
-<iframe src="//${thing.media_domain}/mediaembed/${thing.id36}"
+<iframe src="//${thing.media_domain}/mediaembed/${thing.id36}${thing.credentials}"
id="media-embed-${thing.id36}-${randstr(3)}" class="media-embed"
width="${thing.width}" height="${thing.height}" border="0"
frameBorder="0" scrolling="${'auto' if thing.scrolling else 'no'}"

0 comments on commit a6063c5

Please sign in to comment.