From 77fe0dfa3c52b170d95ab0aad2e568cef96e1160 Mon Sep 17 00:00:00 2001 From: Neil Williams Date: Fri, 14 Feb 2014 16:29:33 -0800 Subject: [PATCH] Require credentials for private subreddit media embeds. 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 makes required the credentials which were added in an earlier patch. This fixes an information disclosure vulnerability reported by Jordan Milne (/u/largenocream). --- r2/r2/controllers/mediaembed.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/r2/r2/controllers/mediaembed.py b/r2/r2/controllers/mediaembed.py index e68d30a2c..775679a65 100644 --- a/r2/r2/controllers/mediaembed.py +++ b/r2/r2/controllers/mediaembed.py @@ -20,23 +20,36 @@ # Inc. All Rights Reserved. ############################################################################### +import hashlib +import hmac + from pylons import request, g, c from pylons.controllers.util import abort from r2.controllers.reddit_base import MinimalController from r2.lib.pages import MediaEmbedBody from r2.lib.media import get_media_embed -from r2.lib.validator import validate, VLink +from r2.lib.utils import constant_time_compare +from r2.lib.validator import validate, VLink, nop class MediaembedController(MinimalController): - @validate(link = VLink('link')) - def GET_mediaembed(self, link): + @validate( + link=VLink('link'), + credentials=nop('credentials'), + ) + def GET_mediaembed(self, link, credentials): if request.host != g.media_domain: # don't serve up untrusted content except on our # specifically untrusted domain abort(404) + if link.subreddit_slow.type == "private": + expected_mac = hmac.new(g.secrets["media_embed"], link._id36, + hashlib.sha1).hexdigest() + if not constant_time_compare(credentials or "", expected_mac): + abort(404) + if not c.secure: media_object = link.media_object else: