From cd518625bf561f1ed42db0b78030b74c32435136 Mon Sep 17 00:00:00 2001 From: redmatrix Date: Sun, 1 May 2016 19:19:17 -0700 Subject: some much needed work on oembed security --- include/oembed.php | 177 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 122 insertions(+), 55 deletions(-) (limited to 'include/oembed.php') diff --git a/include/oembed.php b/include/oembed.php index 3994af0fb..356b9f961 100755 --- a/include/oembed.php +++ b/include/oembed.php @@ -3,67 +3,117 @@ function oembed_replacecb($matches){ $embedurl=$matches[1]; + $action = oembed_action($embedurl); + if($action === 'block') { + return '' . $embedurl . ''; + } + + $j = oembed_fetch_url($embedurl); + $s = oembed_format_object($j); + return $s; +} + + +function oembed_action($embedurl) { + + $host = ''; + + $action = 'allow'; + + // The default action is 'allow'. This is insecure. We might want to + // change this to 'filter' except it will be a support burden because + // then youtube videos won't work out of the box and will need to be + // explicitly enabled. + + $embedurl = str_replace('&','&', $embedurl); + + logger('oembed_action: ' . $embedurl); + + $p = parse_url($embedurl); + + if($p) + $host = $p['host']; + + // These media files should now be caught in bbcode.php + // left here as a fallback in case this is called from another source + + $noexts = array("mp3","mp4","ogg","ogv","oga","ogm","webm","opus"); + $ext = pathinfo(strtolower($embedurl),PATHINFO_EXTENSION); + // site white/black list if(($x = get_config('system','embed_deny'))) { - $l = explode("\n",$x); - if($l) { - foreach($l as $ll) { - if(trim($ll) && strpos($embedurl,trim($ll)) !== false) - return '' . $embedurl . ''; + if(($x) && (! is_array($x))) + $x = explode("\n",$x); + if($x) { + foreach($x as $ll) { + $t = trim($ll); + + // don't allow somebody to provide a url like https://foobar.com/something/youtube + // to bypass a block or allow of youtube + + if($t && (strpos($embedurl,$t) !== false || strpos($t,$host) !== false)) { + $action = 'block'; + break; + } } } } + + $found = false; + if(($x = get_config('system','embed_allow'))) { - $found = false; - $l = explode("\n",$x); - if($l) { - foreach($l as $ll) { - if(trim($ll) && strpos($embedurl,trim($ll)) !== false) { + if(($x) && (! is_array($x))) + $x = explode("\n",$x); + if($x) { + foreach($x as $ll) { + $t = trim($ll); + + // don't allow somebody to provide a url like https://foobar.com/something/youtube + // to bypass a block or allow of youtube + + if($t && (strpos($embedurl,$t) !== false || strpos($t,$host) !== false)) { $found = true; + $action = 'allow'; break; } } } - if(! $found) { - return '' . $embedurl . ''; + if((! $found) && ($action !== 'block')) { + $action = 'filter'; } } - // implements a personal embed white/black list for logged in members + // allow individual members to block something that wasn't blocked already. + // They cannot over-ride the site to allow or change the filtering on an + // embed that is not allowed by the site. + if(local_channel()) { if(($x = get_pconfig(local_channel(),'system','embed_deny'))) { - $l = explode("\n",$x); - if($l) { - foreach($l as $ll) { - if(trim($ll) && strpos($embedurl,trim($ll)) !== false) - return '' . $embedurl . ''; - } - } - } - if(($x = get_pconfig(local_channel(),'system','embed_allow'))) { - $found = false; - $l = explode("\n",$x); - if($l) { - foreach($l as $ll) { - if(trim($ll) && strpos($embedurl,trim($ll)) !== false) { - $found = true; + if(($x) && (! is_array($x))) + $x = explode("\n",$x); + if($x) { + foreach($x as $ll) { + $t = trim($ll); + + // don't allow somebody to provide a url like https://foobar.com/something/youtube + // to bypass a block or allow of youtube + + if($t && (strpos($embedurl,$t) !== false || strpos($t,$host) !== false)) { + $action = 'block'; break; } } } - if(! $found) { - return '' . $embedurl . ''; - } } } - $j = oembed_fetch_url($embedurl); - $s = oembed_format_object($j); - return $s; -} + logger('action: ' . $action . ' url: ' . $embedurl, LOGGER_DEBUG,LOG_DEBUG); + return $action; + +} // if the url is embeddable with oembed, return the bbcode link. @@ -79,42 +129,48 @@ function oembed_process($url) { function oembed_fetch_url($embedurl){ - $a = get_app(); + // These media files should now be caught in bbcode.php + // left here as a fallback in case this is called from another source + + $noexts = array("mp3","mp4","ogg","ogv","oga","ogm","webm","opus"); + $ext = pathinfo(strtolower($embedurl),PATHINFO_EXTENSION); + + $action = oembed_action($embedurl); $embedurl = str_replace('&','&', $embedurl); -// logger('fetch: ' . $embedurl); + $txt = null; - $txt = Cache::get(App::$videowidth . $embedurl); + if($action !== 'block') { + $txt = Cache::get(App::$videowidth . $embedurl); - if(strstr($txt,'youtu') && strstr(z_root(),'https:')) { - $txt = str_replace('http:','https:',$txt); + if(strstr($txt,'youtu') && strstr(z_root(),'https:')) { + $txt = str_replace('http:','https:',$txt); + } } + + if(is_null($txt)) { - // These media files should now be caught in bbcode.php - // left here as a fallback in case this is called from another source - - $noexts = array("mp3","mp4","ogg","ogv","oga","ogm","webm","opus"); - $ext = pathinfo(strtolower($embedurl),PATHINFO_EXTENSION); - - - if(is_null($txt)){ $txt = ""; - - if (in_array($ext, $noexts)) { + $furl = $embedurl; + $zrl = false; + + if(local_channel()) { require_once('include/hubloc.php'); - $zrl = is_matrix_url($embedurl); + $zrl = is_matrix_url($furl); if($zrl) - $embedurl = zid($embedurl); + $furl = zid($furl); } - else { + + + if (! in_array($ext, $noexts) && $action !== 'block') { // try oembed autodiscovery $redirects = 0; - $result = z_fetch_url($embedurl, false, $redirects, array('timeout' => 15, 'accept_content' => "text/*", 'novalidate' => true )); + $result = z_fetch_url($furl, false, $redirects, array('timeout' => 15, 'accept_content' => "text/*", 'novalidate' => true )); if($result['success']) $html_text = $result['body']; - if($html_text){ + if($html_text) { $dom = @DOMDocument::loadHTML($html_text); if ($dom){ $xpath = new DOMXPath($dom); @@ -149,6 +205,7 @@ function oembed_fetch_url($embedurl){ } $txt=trim($txt); + if ($txt[0]!="{") $txt='{"type":"error"}'; //save in cache @@ -160,6 +217,16 @@ function oembed_fetch_url($embedurl){ $j = json_decode($txt); + + if($j->html && $action === 'filter') { + $orig = $j->html; + $allow_position = (($zrl) ? true : false); + $j->html = purify_html($j->html,$allow_position); + if($j->html != $orig) { + logger('oembed html was purified. original: ' . $orig . ' purified: ' . $j->html, LOGGER_DEBUG, LOG_INFO); + } + } + $j->embedurl = $embedurl; // logger('fetch return: ' . print_r($j,true)); -- cgit v1.2.3 From 2b7b26f4c0d6527f9cfe6b852e7b210c0406d9d2 Mon Sep 17 00:00:00 2001 From: redmatrix Date: Mon, 2 May 2016 13:31:14 -0700 Subject: a bit more oembed security - and document the shortcomings of this approach --- include/oembed.php | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) (limited to 'include/oembed.php') diff --git a/include/oembed.php b/include/oembed.php index 356b9f961..af5e51a6f 100755 --- a/include/oembed.php +++ b/include/oembed.php @@ -49,11 +49,7 @@ function oembed_action($embedurl) { if($x) { foreach($x as $ll) { $t = trim($ll); - - // don't allow somebody to provide a url like https://foobar.com/something/youtube - // to bypass a block or allow of youtube - - if($t && (strpos($embedurl,$t) !== false || strpos($t,$host) !== false)) { + if(($t) && (strpos($embedurl,$t) !== false)) { $action = 'block'; break; } @@ -69,14 +65,26 @@ function oembed_action($embedurl) { if($x) { foreach($x as $ll) { $t = trim($ll); + $has_slash = ((strpos($t,'/') !== false) ? true : false); // don't allow somebody to provide a url like https://foobar.com/something/youtube - // to bypass a block or allow of youtube - - if($t && (strpos($embedurl,$t) !== false || strpos($t,$host) !== false)) { - $found = true; - $action = 'allow'; - break; + // to bypass an allow of youtube. Note they could still get through this + // with something like https://youtube.com.foobar.com/something so this is tagged with + // @FIXME, otherwise to fully secure a site will require every possible variation + // of every allowed service base URL. http vs. https, www. vs nothing, + // youtube.[com|org|whatever], youtu.be, and this is just for one service. + + if($t) { + if(strpos($t,$host) !== false) { + $found = true; + $action = 'allow'; + break; + } + elseif(($has_slash) && (strpos($embedurl,$t) !== false)) { + $found = true; + $action = 'allow'; + break; + } } } } @@ -96,11 +104,7 @@ function oembed_action($embedurl) { if($x) { foreach($x as $ll) { $t = trim($ll); - - // don't allow somebody to provide a url like https://foobar.com/something/youtube - // to bypass a block or allow of youtube - - if($t && (strpos($embedurl,$t) !== false || strpos($t,$host) !== false)) { + if(($t) && (strpos($embedurl,$t) !== false)) { $action = 'block'; break; } -- cgit v1.2.3 From b371c028ad31180b4c73f92b45c4ca8f5fff259e Mon Sep 17 00:00:00 2001 From: redmatrix Date: Mon, 2 May 2016 22:28:27 -0700 Subject: more security stuff --- include/oembed.php | 84 +++++++++++++++++++++++------------------------------- 1 file changed, 35 insertions(+), 49 deletions(-) (limited to 'include/oembed.php') diff --git a/include/oembed.php b/include/oembed.php index af5e51a6f..1e5c51172 100755 --- a/include/oembed.php +++ b/include/oembed.php @@ -1,14 +1,16 @@ ' . $embedurl . ''; + $result = oembed_action($embedurl); + if($result['action'] === 'block') { + return '' . $result['url'] . ''; } - $j = oembed_fetch_url($embedurl); + $j = oembed_fetch_url($result['url']); $s = oembed_format_object($j); return $s; } @@ -17,22 +19,11 @@ function oembed_replacecb($matches){ function oembed_action($embedurl) { $host = ''; + $action = 'filter'; - $action = 'allow'; - - // The default action is 'allow'. This is insecure. We might want to - // change this to 'filter' except it will be a support burden because - // then youtube videos won't work out of the box and will need to be - // explicitly enabled. - - $embedurl = str_replace('&','&', $embedurl); + $embedurl = trim(str_replace('&','&', $embedurl)); - logger('oembed_action: ' . $embedurl); - - $p = parse_url($embedurl); - - if($p) - $host = $p['host']; + logger('oembed_action: ' . $embedurl, LOGGER_DEBUG, LOG_INFO); // These media files should now be caught in bbcode.php // left here as a fallback in case this is called from another source @@ -40,6 +31,11 @@ function oembed_action($embedurl) { $noexts = array("mp3","mp4","ogg","ogv","oga","ogm","webm","opus"); $ext = pathinfo(strtolower($embedurl),PATHINFO_EXTENSION); + if(strpos($embedurl,'http://') === 0) { + if(intval(get_config('system','embed_sslonly'))) { + $action = 'block'; + } + } // site white/black list @@ -65,26 +61,10 @@ function oembed_action($embedurl) { if($x) { foreach($x as $ll) { $t = trim($ll); - $has_slash = ((strpos($t,'/') !== false) ? true : false); - - // don't allow somebody to provide a url like https://foobar.com/something/youtube - // to bypass an allow of youtube. Note they could still get through this - // with something like https://youtube.com.foobar.com/something so this is tagged with - // @FIXME, otherwise to fully secure a site will require every possible variation - // of every allowed service base URL. http vs. https, www. vs nothing, - // youtube.[com|org|whatever], youtu.be, and this is just for one service. - - if($t) { - if(strpos($t,$host) !== false) { - $found = true; - $action = 'allow'; - break; - } - elseif(($has_slash) && (strpos($embedurl,$t) !== false)) { - $found = true; - $action = 'allow'; - break; - } + if(($t) && (strpos($embedurl,$t) !== false) && ($action !== 'block')) { + $found = true; + $action = 'allow'; + break; } } } @@ -95,7 +75,7 @@ function oembed_action($embedurl) { // allow individual members to block something that wasn't blocked already. // They cannot over-ride the site to allow or change the filtering on an - // embed that is not allowed by the site. + // embed that is not allowed by the site admin. if(local_channel()) { if(($x = get_pconfig(local_channel(),'system','embed_deny'))) { @@ -113,9 +93,12 @@ function oembed_action($embedurl) { } } - logger('action: ' . $action . ' url: ' . $embedurl, LOGGER_DEBUG,LOG_DEBUG); + $arr = array('url' => $embedurl, 'action' => $action); + call_hooks('oembed_action',$arr); + + logger('action: ' . $arr['action'] . ' url: ' . $arr['url'], LOGGER_DEBUG,LOG_DEBUG); - return $action; + return $arr; } @@ -139,9 +122,10 @@ function oembed_fetch_url($embedurl){ $noexts = array("mp3","mp4","ogg","ogv","oga","ogm","webm","opus"); $ext = pathinfo(strtolower($embedurl),PATHINFO_EXTENSION); - $action = oembed_action($embedurl); + $result = oembed_action($embedurl); - $embedurl = str_replace('&','&', $embedurl); + $embedurl = $result['url']; + $action = $result['action']; $txt = null; @@ -222,12 +206,14 @@ function oembed_fetch_url($embedurl){ $j = json_decode($txt); - if($j->html && $action === 'filter') { - $orig = $j->html; - $allow_position = (($zrl) ? true : false); - $j->html = purify_html($j->html,$allow_position); - if($j->html != $orig) { - logger('oembed html was purified. original: ' . $orig . ' purified: ' . $j->html, LOGGER_DEBUG, LOG_INFO); + if($action === 'filter') { + if($j->html) { + $orig = $j->html; + $allow_position = (($zrl) ? true : false); + $j->html = purify_html($j->html,$allow_position); + if($j->html != $orig) { + logger('oembed html was purified. original: ' . $orig . ' purified: ' . $j->html, LOGGER_DEBUG, LOG_INFO); + } } } -- cgit v1.2.3