Просмотр исходного кода

UI: Properly verify state parameter for YouTube auth

Very low risk of anything bad here since we use a random port and the
chance of a CSRF attack is tiny, but this is a best practie to do when
using OAuth.
Richard Stanway 4 лет назад
Родитель
Сommit
0a13ce851d
3 измененных файлов с 39 добавлено и 6 удалено
  1. 32 5
      UI/auth-listener.cpp
  2. 2 0
      UI/auth-listener.hpp
  3. 5 1
      UI/auth-youtube.cpp

+ 32 - 5
UI/auth-listener.cpp

@@ -45,6 +45,11 @@ quint16 AuthListener::GetPort()
 	return server ? server->serverPort() : 0;
 }
 
+void AuthListener::SetState(QString state)
+{
+	this->state = state;
+}
+
 void AuthListener::NewConnection()
 {
 	QTcpSocket *socket = server->nextPendingConnection();
@@ -60,12 +65,34 @@ void AuthListener::NewConnection()
 			QString redirect = QString::fromLatin1(buffer);
 			blog(LOG_DEBUG, "redirect: %s", QT_TO_UTF8(redirect));
 
-			QRegularExpression re("(&|\\?)code=(?<code>[^&]+)");
-			QRegularExpressionMatch match = re.match(redirect);
-			if (!match.hasMatch())
-				blog(LOG_DEBUG, "no 'code' in server redirect");
+			QRegularExpression re_state(
+				"(&|\\?)state=(?<state>[^&]+)");
+			QRegularExpression re_code(
+				"(&|\\?)code=(?<code>[^&]+)");
+
+			QRegularExpressionMatch match =
+				re_state.match(redirect);
 
-			QString code = match.captured("code");
+			QString code;
+
+			if (match.hasMatch()) {
+				if (state == match.captured("state")) {
+					match = re_code.match(redirect);
+					if (!match.hasMatch())
+						blog(LOG_DEBUG, "no 'code' "
+								"in server "
+								"redirect");
+
+					code = match.captured("code");
+				} else {
+					blog(LOG_WARNING, "state mismatch "
+							  "while handling "
+							  "redirect");
+				}
+			} else {
+				blog(LOG_DEBUG, "no 'state' in "
+						"server redirect");
+			}
 
 			if (code.isEmpty()) {
 				auto data = QTStr("YouTube.Auth.NoCode");

+ 2 - 0
UI/auth-listener.hpp

@@ -7,6 +7,7 @@ class AuthListener : public QObject {
 	Q_OBJECT
 
 	QTcpServer *server;
+	QString state;
 
 signals:
 	void ok(const QString &code);
@@ -18,4 +19,5 @@ protected:
 public:
 	explicit AuthListener(QObject *parent = 0);
 	quint16 GetPort();
+	void SetState(QString state);
 };

+ 5 - 1
UI/auth-youtube.cpp

@@ -231,6 +231,10 @@ std::shared_ptr<Auth> YoutubeAuth::Login(QWidget *owner,
 	deobfuscate_str(&clientid[0], YOUTUBE_CLIENTID_HASH);
 	deobfuscate_str(&secret[0], YOUTUBE_SECRET_HASH);
 
+	QString state;
+	state = auth->GenerateState();
+	server.SetState(state);
+
 	QString url_template;
 	url_template += "%1";
 	url_template += "?response_type=code";
@@ -239,7 +243,7 @@ std::shared_ptr<Auth> YoutubeAuth::Login(QWidget *owner,
 	url_template += "&state=%4";
 	url_template += "&scope=https://www.googleapis.com/auth/youtube";
 	QString url = url_template.arg(YOUTUBE_AUTH_URL, clientid.c_str(),
-				       redirect_uri, auth->GenerateState());
+				       redirect_uri, state);
 
 	QString text = QTStr("YouTube.Auth.WaitingAuth.Text");
 	text = text.arg(