SECURITY: Sanitize YouTube Onebox data (#13748)

CVE-2021-32764

lib/onebox/engine/youtube_onebox.rb

@@ -89,25 +89,31 @@ module Onebox
 
       def video_id
         @video_id ||= begin
+          id = nil
+
           # http://youtu.be/afyK1HSFfgw
           if uri.host["youtu.be"]
             id = uri.path[/\/([\w\-]+)/, 1]
-            return id if id
           end
 
           # https://www.youtube.com/embed/vsF0K3Ou1v0
           if uri.path["/embed/"]
-            id = uri.path[/\/embed\/([\w\-]+)/, 1]
-            return id if id
+            id ||= uri.path[/\/embed\/([\w\-]+)/, 1]
           end
 
           # https://www.youtube.com/watch?v=Z0UISCEe52Y
-          params['v']
+          id ||= params['v']
+
+          sanitize_yt_id(id)
         end
       end
 
       def list_id
-        @list_id ||= params['list']
+        @list_id ||= sanitize_yt_id(params['list'])
+      end
+
+      def sanitize_yt_id(raw)
+        raw&.match?(/\A[\w-]+\z/) ? raw : nil
       end
 
       def embed_params

plugins/lazy-yt/assets/javascripts/initializers/lazyYT.js

@@ -1,9 +1,11 @@
 import { withPluginApi } from "discourse/lib/plugin-api";
+import initLazyYt from "../lib/lazyYT";
 
 export default {
   name: "apply-lazyYT",
   initialize() {
     withPluginApi("0.1", (api) => {
+      initLazyYt($);
       api.decorateCooked(
         ($elem) => {
           const iframes = $(".lazyYT", $elem);

plugins/lazy-yt/assets/javascripts/lib/lazyYT.js

@@ -11,7 +11,9 @@
  *
  */
 
-(function ($) {
+import escape from "discourse-common/lib/escape";
+
+export default function initLazyYt($) {
   "use strict";
 
   function setUp($el, settings) {
@@ -75,13 +77,13 @@
     innerHtml.push('<div class="html5-title-text-wrapper">');
     innerHtml.push(
       '<a class="html5-title-text" target="_blank" tabindex="3100" href="https://www.youtube.com/watch?v=',
-      id,
+      escape(id),
       '">'
     );
     if (title === undefined || title === null || title === "") {
-      innerHtml.push("youtube.com/watch?v=" + id);
+      innerHtml.push("youtube.com/watch?v=" + escape(id));
     } else {
-      innerHtml.push(title);
+      innerHtml.push(escape(title));
     }
     innerHtml.push("</a>");
     innerHtml.push("</div>"); // .html5-title
@@ -121,7 +123,7 @@
           $(
             [
               '<img class="ytp-thumbnail-image" src="https://img.youtube.com/vi/',
-              id,
+              escape(id),
               "/",
               thumb_img,
               '">',
@@ -143,7 +145,7 @@
           $el
             .html(
               '<iframe src="//www.youtube.com/embed/' +
-                id +
+                escape(id) +
                 "?autoplay=1&" +
                 youtube_parameters +
                 '" frameborder="0" allowfullscreen></iframe>'
@@ -170,4 +172,4 @@
       setUp($el, settings);
     });
   };
-})(jQuery);
+}

plugins/lazy-yt/plugin.rb

@@ -5,14 +5,12 @@
 # version: 1.0.1
 # authors: Arpit Jalan
 # url: https://github.com/discourse/discourse/tree/master/plugins/lazy-yt
+# transpile_js: true
 
 hide_plugin if self.respond_to?(:hide_plugin)
 
 require "onebox"
 
-# javascript
-register_asset "javascripts/lazyYT.js"
-
 # stylesheet
 register_asset "stylesheets/lazyYT.css"
 register_asset "stylesheets/lazyYT_mobile.scss", :mobile

spec/lib/onebox/engine/youtube_onebox_spec.rb

@@ -65,6 +65,17 @@ describe Onebox::Engine::YoutubeOnebox do
     expect(Onebox.preview('https://www.youtube.com/watch?v=21Lk4YiASMo&potential[]=exploit&potential[]=fun').to_s).not_to match(/potential|exploit|fun/)
   end
 
+  it "ignores video_id with unacceptable characters" do
+    # (falls back to generic onebox)
+    Onebox::Engine::AllowlistedGenericOnebox.any_instance.stubs(:to_html).returns(+"allowlisted_html")
+    expect(Onebox.preview('https://www.youtube.com/watch?v=%3C%3E21Lk4YiASMo').to_s).to eq("allowlisted_html")
+  end
+
+  it "ignores list_id with unacceptable characters" do
+    # (falls back to video-only onebox)
+    expect(Onebox.preview('https://www.youtube.com/watch?v=21Lk4YiASMo&list=%3C%3EUUQau-O2C0kGJpR3_CHBTGbw').to_s).not_to include("UUQau-O2C0kGJpR3_CHBTGbw")
+  end
+
   it "converts time strings into a &start= parameter" do
     expect(Onebox.preview('https://www.youtube.com/watch?v=21Lk4YiASMo&start=3782').to_s).to match(/start=3782/)
     expect(Onebox.preview('https://www.youtube.com/watch?start=1h3m2s&v=21Lk4YiASMo').to_s).to match(/start=3782/)