From 26168a9520c8bc2a03f631eb0e53c3ee086d8c98 Mon Sep 17 00:00:00 2001 From: Omar Roth Date: Mon, 15 Apr 2019 23:23:40 -0500 Subject: [PATCH] Refactor CSRF tokens (using format in #473) --- src/invidious.cr | 169 +++++------ src/invidious/users.cr | 73 +++-- src/invidious/views/clear_watch_history.ecr | 3 +- src/invidious/views/components/item.ecr | 24 +- .../views/components/subscribe_widget.ecr | 18 +- .../components/subscribe_widget_script.ecr | 10 +- src/invidious/views/delete_account.ecr | 3 +- src/invidious/views/history.ecr | 27 +- src/invidious/views/login.ecr | 6 +- src/invidious/views/subscription_manager.ecr | 21 +- src/invidious/views/subscriptions.ecr | 9 +- src/invidious/views/template.ecr | 263 +++++++++--------- 12 files changed, 321 insertions(+), 305 deletions(-) diff --git a/src/invidious.cr b/src/invidious.cr index b0b512fb..8ed4253f 100644 --- a/src/invidious.cr +++ b/src/invidious.cr @@ -195,39 +195,33 @@ before_all do |env| end if env.request.cookies.has_key? "SID" - headers = HTTP::Headers.new - headers["Cookie"] = env.request.headers["Cookie"] - sid = env.request.cookies["SID"].value # Invidious users only have SID if !env.request.cookies.has_key? "SSID" - email = PG_DB.query_one?("SELECT email FROM session_ids WHERE id = $1", sid, as: String) - - if email + if email = PG_DB.query_one?("SELECT email FROM session_ids WHERE id = $1", sid, as: String) user = PG_DB.query_one("SELECT * FROM users WHERE email = $1", email, as: User) - challenge, token = create_response(user.email, "sign_out", HMAC_KEY, PG_DB, 1.week) - - env.set "challenge", challenge - env.set "token", token + token = create_response(sid, {"signout", "watch_ajax", "subscription_ajax"}, HMAC_KEY, PG_DB, 1.week) preferences = user.preferences - env.set "user", user env.set "sid", sid + env.set "token", token + env.set "user", user end else + headers = HTTP::Headers.new + headers["Cookie"] = env.request.headers["Cookie"] + begin user, sid = get_user(sid, headers, PG_DB, false) - - challenge, token = create_response(user.email, "sign_out", HMAC_KEY, PG_DB, 1.week) - env.set "challenge", challenge - env.set "token", token + token = create_response(sid, {"signout", "watch_ajax", "subscription_ajax"}, HMAC_KEY, PG_DB, 1.week) preferences = user.preferences - env.set "user", user env.set "sid", sid + env.set "token", token + env.set "user", user rescue ex end end @@ -826,7 +820,7 @@ post "/login" do |env| when "google" tfa_code = env.params.body["tfa"]?.try &.lchop("G-") - # See https://github.com/rg3/youtube-dl/blob/master/youtube_dl/extractor/youtube.py#L79 + # See https://github.com/ytdl-org/youtube-dl/blob/2019.04.07/youtube_dl/extractor/youtube.py#L82 begin client = make_client(LOGIN_URL) headers = HTTP::Headers.new @@ -1091,8 +1085,7 @@ post "/login" do |env| next templated "login" end - challenges = env.params.body.select { |k, v| k.match(/^challenge\[\d+\]$/) } - tokens = env.params.body.select { |k, v| k.match(/^token\[\d+\]$/) } + tokens = env.params.body.select { |k, v| k.match(/^token\[\d+\]$/) }.map { |k, v| v } answer ||= "" captcha_type ||= "image" @@ -1102,11 +1095,8 @@ post "/login" do |env| answer = answer.lstrip('0') answer = OpenSSL::HMAC.hexdigest(:sha256, HMAC_KEY, answer) - challenge = env.params.body["challenge[0]"]? - token = env.params.body["token[0]"]? - begin - validate_response(challenge, token, answer, "sign_in", HMAC_KEY, PG_DB, locale) + validate_response(tokens[0], answer, env.request.path, HMAC_KEY, PG_DB, locale) rescue ex error_message = ex.message next templated "error" @@ -1117,11 +1107,9 @@ post "/login" do |env| found_valid_captcha = false error_message = translate(locale, "Invalid CAPTCHA") - challenges.each_with_index do |challenge, i| + tokens.each_with_index do |token, i| begin - challenge = challenge[1] - token = tokens[i][1] - validate_response(challenge, token, answer, "sign_in", HMAC_KEY, PG_DB, locale) + validate_response(token, answer, env.request.path, HMAC_KEY, PG_DB, locale) found_valid_captcha = true rescue ex error_message = ex.message @@ -1191,27 +1179,25 @@ post "/login" do |env| end end -get "/signout" do |env| +post "/signout" do |env| locale = LOCALES[env.get("preferences").as(Preferences).locale]? user = env.get? "user" + sid = env.get? "sid" referer = get_referer(env) if user user = user.as(User) - - challenge = env.params.query["challenge"]? - token = env.params.query["token"]? + sid = sid.as(String) + token = env.params.body["token"]? begin - validate_response(challenge, token, user.email, "sign_out", HMAC_KEY, PG_DB, locale) + validate_response(token, sid, env.request.path, HMAC_KEY, PG_DB, locale) rescue ex error_message = ex.message next templated "error" end - user = env.get("user").as(User) - sid = env.get("sid").as(String) PG_DB.exec("DELETE FROM session_ids * WHERE id = $1", sid) env.request.cookies.each do |cookie| @@ -1426,42 +1412,24 @@ get "/toggle_theme" do |env| env.redirect referer end -get "/mark_watched" do |env| +post "/watch_ajax" do |env| locale = LOCALES[env.get("preferences").as(Preferences).locale]? user = env.get? "user" + sid = env.get? "sid" referer = get_referer(env, "/feed/subscriptions") - id = env.params.query["id"]? - if !id - env.response.status_code = 400 - next - end - redirect = env.params.query["redirect"]? - redirect ||= "false" + redirect ||= "true" redirect = redirect == "true" - if user - user = user.as(User) - if !user.watched.includes? id - PG_DB.exec("UPDATE users SET watched = watched || $1 WHERE email = $2", [id], user.email) - end - end - - if redirect - env.redirect referer - else - env.response.content_type = "application/json" - "{}" + if !user + next env.redirect referer end -end -get "/mark_unwatched" do |env| - locale = LOCALES[env.get("preferences").as(Preferences).locale]? - - user = env.get? "user" - referer = get_referer(env, "/feed/history") + user = user.as(User) + sid = sid.as(String) + token = env.params.body["token"]? id = env.params.query["id"]? if !id @@ -1469,12 +1437,37 @@ get "/mark_unwatched" do |env| next end - redirect = env.params.query["redirect"]? - redirect ||= "false" - redirect = redirect == "true" + user = user.as(User) + sid = sid.as(String) + token = env.params.body["token"]? - if user - user = user.as(User) + begin + validate_response(token, sid, env.request.path, HMAC_KEY, PG_DB, locale) + rescue ex + if redirect + error_message = ex.message + next templated "error" + else + error_message = {"error" => ex.message}.to_json + env.response.status_code = 500 + next error_message + end + end + + if env.params.query["action_mark_watched"]? + action = "action_mark_watched" + elsif env.params.query["action_mark_unwatched"]? + action = "action_mark_unwatched" + else + next env.redirect referer + end + + case action + when "action_mark_watched" + if !user.watched.includes? id + PG_DB.exec("UPDATE users SET watched = watched || $1 WHERE email = $2", [id], user.email) + end + when "action_mark_unwatched" PG_DB.exec("UPDATE users SET watched = array_remove(watched, $1) WHERE email = $2", id, user.email) end @@ -1561,8 +1554,7 @@ get "/modify_notifications" do |env| end end -# TODO: Add CSRF -get "/subscription_ajax" do |env| +post "/subscription_ajax" do |env| locale = LOCALES[env.get("preferences").as(Preferences).locale]? user = env.get? "user" @@ -1570,14 +1562,29 @@ get "/subscription_ajax" do |env| referer = get_referer(env, "/") redirect = env.params.query["redirect"]? - redirect ||= "false" + redirect ||= "true" redirect = redirect == "true" - if !user && !sid + if !user next env.redirect referer end user = user.as(User) + sid = sid.as(String) + token = env.params.body["token"]? + + begin + validate_response(token, sid, env.request.path, HMAC_KEY, PG_DB, locale) + rescue ex + if redirect + error_message = ex.message + next templated "error" + else + error_message = {"error" => ex.message}.to_json + env.response.status_code = 500 + next error_message + end + end if env.params.query["action_create_subscription_to_channel"]? action = "action_create_subscription_to_channel" @@ -1653,7 +1660,7 @@ get "/subscription_manager" do |env| user = env.get? "user" sid = env.get? "sid" - referer = get_referer(env, "/") + referer = get_referer(env, "/subscription_manager") if !user && !sid next env.redirect referer @@ -1843,12 +1850,13 @@ get "/delete_account" do |env| locale = LOCALES[env.get("preferences").as(Preferences).locale]? user = env.get? "user" + sid = env.get? "sid" referer = get_referer(env) if user user = user.as(User) - - challenge, token = create_response(user.email, "delete_account", HMAC_KEY, PG_DB) + sid = sid.as(String) + token = create_response(sid, {"delete_account"}, HMAC_KEY, PG_DB) templated "delete_account" else @@ -1860,16 +1868,16 @@ post "/delete_account" do |env| locale = LOCALES[env.get("preferences").as(Preferences).locale]? user = env.get? "user" + sid = env.get? "sid" referer = get_referer(env) if user user = user.as(User) - - challenge = env.params.body["challenge"]? + sid = sid.as(String) token = env.params.body["token"]? begin - validate_response(challenge, token, user.email, "delete_account", HMAC_KEY, PG_DB, locale) + validate_response(token, sid, env.request.path, HMAC_KEY, PG_DB, locale) rescue ex error_message = ex.message next templated "error" @@ -1893,12 +1901,13 @@ get "/clear_watch_history" do |env| locale = LOCALES[env.get("preferences").as(Preferences).locale]? user = env.get? "user" + sid = env.get? "sid" referer = get_referer(env) if user user = user.as(User) - - challenge, token = create_response(user.email, "clear_watch_history", HMAC_KEY, PG_DB) + sid = sid.as(String) + token = create_response(sid, {"clear_watch_history"}, HMAC_KEY, PG_DB) templated "clear_watch_history" else @@ -1910,16 +1919,16 @@ post "/clear_watch_history" do |env| locale = LOCALES[env.get("preferences").as(Preferences).locale]? user = env.get? "user" + sid = env.get? "sid" referer = get_referer(env) if user user = user.as(User) - - challenge = env.params.body["challenge"]? + sid = sid.as(String) token = env.params.body["token"]? begin - validate_response(challenge, token, user.email, "clear_watch_history", HMAC_KEY, PG_DB, locale) + validate_response(token, sid, env.request.path, HMAC_KEY, PG_DB, locale) rescue ex error_message = ex.message next templated "error" diff --git a/src/invidious/users.cr b/src/invidious/users.cr index f34d14ab..bad2b5c3 100644 --- a/src/invidious/users.cr +++ b/src/invidious/users.cr @@ -197,84 +197,79 @@ def create_user(sid, email, password) return user, sid end -def create_response(user_id, operation, key, db, expire = 6.hours) +def create_response(session, scopes, key, db, expire = 6.hours, use_nonce = false) expire = Time.now + expire - nonce = Random::Secure.hex(16) - db.exec("INSERT INTO nonces VALUES ($1, $2) ON CONFLICT DO NOTHING", nonce, expire) - challenge = "#{expire.to_unix}-#{nonce}-#{user_id}-#{operation}" - token = OpenSSL::HMAC.digest(:sha256, key, challenge) + token = { + "session" => session, + "expire" => expire.to_unix, + "scopes" => scopes, + } + + if use_nonce + nonce = Random::Secure.hex(16) + db.exec("INSERT INTO nonces VALUES ($1, $2) ON CONFLICT DO NOTHING", nonce, expire) + token["nonce"] = nonce + end - challenge = Base64.urlsafe_encode(challenge) - token = Base64.urlsafe_encode(token) + token["signature"] = sign_token(key, token) - return challenge, token + return token.to_json end def sign_token(key, hash) string_to_sign = [] of String + hash.each do |key, value| if key == "signature" next end + if value.is_a?(JSON::Any) + case value + when .as_a? + value = value.as_a.map { |item| item.as_s } + end + end + case value when Array string_to_sign << "#{key}=#{value.sort.join(",")}" + when Tuple + string_to_sign << "#{key}=#{value.to_a.sort.join(",")}" else string_to_sign << "#{key}=#{value}" end end string_to_sign = string_to_sign.sort.join("\n") - return Base64.encode(OpenSSL::HMAC.digest(:sha256, key, string_to_sign)).strip + return Base64.urlsafe_encode(OpenSSL::HMAC.digest(:sha256, key, string_to_sign)).strip end -def validate_response(challenge, token, user_id, operation, key, db, locale) - if !challenge - raise translate(locale, "Hidden field \"challenge\" is a required field") - end - +def validate_response(token, session, scope, key, db, locale) if !token raise translate(locale, "Hidden field \"token\" is a required field") end - challenge = Base64.decode_string(challenge) - if challenge.split("-").size == 4 - expire, nonce, challenge_user_id, challenge_operation = challenge.split("-") + token = JSON.parse(URI.unescape(token)).as_h - expire = expire.to_i? - expire ||= 0 - else - raise translate(locale, "Invalid challenge") + if token["signature"]? != sign_token(key, token) + raise translate(locale, "Invalid token") end - challenge = OpenSSL::HMAC.digest(:sha256, key, challenge) - challenge = Base64.urlsafe_encode(challenge) - - if nonce = db.query_one?("SELECT * FROM nonces WHERE nonce = $1", nonce, as: {String, Time}) + if token["nonce"]? && (nonce = db.query_one?("SELECT * FROM nonces WHERE nonce = $1", token["nonce"], as: {String, Time})) if nonce[1] > Time.now db.exec("UPDATE nonces SET expire = $1 WHERE nonce = $2", Time.new(1990, 1, 1), nonce[0]) else raise translate(locale, "Invalid token") end - else - raise translate(locale, "Invalid token") - end - - if challenge != token - raise translate(locale, "Invalid token") - end - - if challenge_operation != operation - raise translate(locale, "Invalid token") end - if challenge_user_id != user_id + if !token["scopes"].as_a.includes? scope.strip("/") raise translate(locale, "Invalid token") end - if expire < Time.now.to_unix + if token["expire"].as_i < Time.now.to_unix raise translate(locale, "Token is expired, please try again") end end @@ -331,7 +326,7 @@ def generate_captcha(key, db) return { question: image, - tokens: [create_response(answer, "sign_in", key, db)], + tokens: {create_response(answer, {"login"}, key, db, use_nonce: true)}, } end @@ -340,7 +335,7 @@ def generate_text_captcha(key, db) response = JSON.parse(response) tokens = response["a"].as_a.map do |answer| - create_response(answer.as_s, "sign_in", key, db) + create_response(answer.as_s, {"login"}, key, db, use_nonce: true) end return { diff --git a/src/invidious/views/clear_watch_history.ecr b/src/invidious/views/clear_watch_history.ecr index ede5e287..55555bee 100644 --- a/src/invidious/views/clear_watch_history.ecr +++ b/src/invidious/views/clear_watch_history.ecr @@ -19,7 +19,6 @@ - - + diff --git a/src/invidious/views/components/item.ecr b/src/invidious/views/components/item.ecr index 2dc0bea4..ae1ccb96 100644 --- a/src/invidious/views/components/item.ecr +++ b/src/invidious/views/components/item.ecr @@ -85,17 +85,19 @@
<% if env.get? "show_watched" %> -

- - - - -

+
" method="post"> + "> +

+ + + +

+
<% end %> <% if item.responds_to?(:live_now) && item.live_now %>

<%= translate(locale, "LIVE") %>

diff --git a/src/invidious/views/components/subscribe_widget.ecr b/src/invidious/views/components/subscribe_widget.ecr index df16658d..59849c1a 100644 --- a/src/invidious/views/components/subscribe_widget.ecr +++ b/src/invidious/views/components/subscribe_widget.ecr @@ -1,17 +1,21 @@ <% if user %> <% if subscriptions.includes? ucid %>

- "> - <%= translate(locale, "Unsubscribe") %> | <%= sub_count_text %> - +

" method="post"> + "> + + | <%= sub_count_text %>"> + +

<% else %>

- "> - <%= translate(locale, "Subscribe") %> | <%= sub_count_text %> +

" method="post"> + "> + + | <%= sub_count_text %>"> +

<% end %> <% else %> diff --git a/src/invidious/views/components/subscribe_widget_script.ecr b/src/invidious/views/components/subscribe_widget_script.ecr index 9bfd8ebb..8fe0a653 100644 --- a/src/invidious/views/components/subscribe_widget_script.ecr +++ b/src/invidious/views/components/subscribe_widget_script.ecr @@ -15,8 +15,9 @@ function subscribe(timeouts = 0) { var xhr = new XMLHttpRequest(); xhr.responseType = "json"; xhr.timeout = 20000; - xhr.open("GET", url, true); - xhr.send(); + xhr.open("POST", url, true); + xhr.setRequestHeader("Content-Type", "application/x-www-form-urlencoded"); + xhr.send("token=<%= URI.escape(env.get?("token").try &.as(String) || "") %>"); var fallback = subscribe_button.innerHTML; subscribe_button.onclick = unsubscribe; @@ -50,8 +51,9 @@ function unsubscribe(timeouts = 0) { var xhr = new XMLHttpRequest(); xhr.responseType = "json"; xhr.timeout = 20000; - xhr.open("GET", url, true); - xhr.send(); + xhr.open("POST", url, true); + xhr.setRequestHeader("Content-Type", "application/x-www-form-urlencoded"); + xhr.send("token=<%= URI.escape(env.get?("token").try &.as(String) || "") %>"); var fallback = subscribe_button.innerHTML; subscribe_button.onclick = subscribe; diff --git a/src/invidious/views/delete_account.ecr b/src/invidious/views/delete_account.ecr index 7cc8de9b..f49eaab9 100644 --- a/src/invidious/views/delete_account.ecr +++ b/src/invidious/views/delete_account.ecr @@ -19,7 +19,6 @@
- - + diff --git a/src/invidious/views/history.ecr b/src/invidious/views/history.ecr index 9be40a0d..15a24f4d 100644 --- a/src/invidious/views/history.ecr +++ b/src/invidious/views/history.ecr @@ -28,14 +28,16 @@ <% else %>
-

- - - -

+
" method="post"> + "> +

+ + + +

+

<% end %> @@ -48,17 +50,18 @@