mirror of
https://github.com/electron/electron.git
synced 2026-04-10 03:01:51 -04:00
fix: validate protocol scheme names in setAsDefaultProtocolClient (#50156)
fix: validate protocol scheme names in setAsDefaultProtocolClient On Windows, `app.setAsDefaultProtocolClient(protocol)` directly concatenates the protocol string into the registry key path with no validation. A protocol name containing `\` could write to an arbitrary subkey under `HKCU\Software\Classes\`, potentially hijacking existing protocol handlers. To fix this, add `Browser::IsValidProtocolScheme()` which validates that a protocol name conforms to the RFC 3986 scheme grammar: scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ) This rejects backslashes, forward slashes, whitespace, and any other characters not permitted in URI schemes. Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This commit is contained in:
@@ -9,7 +9,9 @@
|
||||
#include <utility>
|
||||
|
||||
#include "base/files/file_util.h"
|
||||
#include "base/logging.h"
|
||||
#include "base/path_service.h"
|
||||
#include "base/strings/string_util.h"
|
||||
#include "base/task/single_thread_task_runner.h"
|
||||
#include "base/threading/thread_restrictions.h"
|
||||
#include "chrome/common/chrome_paths.h"
|
||||
@@ -71,6 +73,29 @@ Browser* Browser::Get() {
|
||||
return ElectronBrowserMainParts::Get()->browser();
|
||||
}
|
||||
|
||||
// static
|
||||
bool Browser::IsValidProtocolScheme(const std::string& scheme) {
|
||||
// RFC 3986 Section 3.1:
|
||||
// scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
|
||||
if (scheme.empty()) {
|
||||
LOG(ERROR) << "Protocol scheme must not be empty";
|
||||
return false;
|
||||
}
|
||||
if (!base::IsAsciiAlpha(scheme[0])) {
|
||||
LOG(ERROR) << "Protocol scheme must start with an ASCII letter";
|
||||
return false;
|
||||
}
|
||||
for (size_t i = 1; i < scheme.size(); ++i) {
|
||||
const char c = scheme[i];
|
||||
if (!base::IsAsciiAlpha(c) && !base::IsAsciiDigit(c) && c != '+' &&
|
||||
c != '-' && c != '.') {
|
||||
LOG(ERROR) << "Protocol scheme contains invalid character: '" << c << "'";
|
||||
return false;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX)
|
||||
void Browser::Focus(gin::Arguments* args) {
|
||||
// Focus on the first visible window.
|
||||
|
||||
@@ -134,6 +134,10 @@ class Browser : private WindowListObserver {
|
||||
void SetAppUserModelID(const std::wstring& name);
|
||||
#endif
|
||||
|
||||
// Validate that a protocol scheme conforms to RFC 3986:
|
||||
// scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
|
||||
static bool IsValidProtocolScheme(const std::string& scheme);
|
||||
|
||||
// Remove the default protocol handler registry key
|
||||
bool RemoveAsDefaultProtocolClient(const std::string& protocol,
|
||||
gin::Arguments* args);
|
||||
|
||||
@@ -103,16 +103,19 @@ void Browser::ClearRecentDocuments() {}
|
||||
|
||||
bool Browser::SetAsDefaultProtocolClient(const std::string& protocol,
|
||||
gin::Arguments* args) {
|
||||
if (!IsValidProtocolScheme(protocol))
|
||||
return false;
|
||||
|
||||
return SetDefaultWebClient(protocol);
|
||||
}
|
||||
|
||||
bool Browser::IsDefaultProtocolClient(const std::string& protocol,
|
||||
gin::Arguments* args) {
|
||||
auto env = base::Environment::Create();
|
||||
|
||||
if (protocol.empty())
|
||||
if (!IsValidProtocolScheme(protocol))
|
||||
return false;
|
||||
|
||||
auto env = base::Environment::Create();
|
||||
|
||||
std::vector<std::string> argv = {kXdgSettings, "check",
|
||||
kXdgSettingsDefaultSchemeHandler, protocol};
|
||||
if (std::optional<std::string> desktop_name = env->GetVar("CHROME_DESKTOP")) {
|
||||
|
||||
@@ -236,7 +236,7 @@ bool Browser::RemoveAsDefaultProtocolClient(const std::string& protocol,
|
||||
|
||||
bool Browser::SetAsDefaultProtocolClient(const std::string& protocol,
|
||||
gin::Arguments* args) {
|
||||
if (protocol.empty())
|
||||
if (!IsValidProtocolScheme(protocol))
|
||||
return false;
|
||||
|
||||
NSString* identifier = [base::apple::MainBundle() bundleIdentifier];
|
||||
@@ -252,7 +252,7 @@ bool Browser::SetAsDefaultProtocolClient(const std::string& protocol,
|
||||
|
||||
bool Browser::IsDefaultProtocolClient(const std::string& protocol,
|
||||
gin::Arguments* args) {
|
||||
if (protocol.empty())
|
||||
if (!IsValidProtocolScheme(protocol))
|
||||
return false;
|
||||
|
||||
NSString* identifier = [base::apple::MainBundle() bundleIdentifier];
|
||||
|
||||
@@ -429,7 +429,7 @@ bool Browser::SetUserTasks(const std::vector<UserTask>& tasks) {
|
||||
|
||||
bool Browser::RemoveAsDefaultProtocolClient(const std::string& protocol,
|
||||
gin::Arguments* args) {
|
||||
if (protocol.empty())
|
||||
if (!IsValidProtocolScheme(protocol))
|
||||
return false;
|
||||
|
||||
// Main Registry Key
|
||||
@@ -508,7 +508,7 @@ bool Browser::SetAsDefaultProtocolClient(const std::string& protocol,
|
||||
// Software\Classes", which is inherited by "HKEY_CLASSES_ROOT"
|
||||
// anyway, and can be written by unprivileged users.
|
||||
|
||||
if (protocol.empty())
|
||||
if (!IsValidProtocolScheme(protocol))
|
||||
return false;
|
||||
|
||||
std::wstring exe;
|
||||
@@ -538,7 +538,7 @@ bool Browser::SetAsDefaultProtocolClient(const std::string& protocol,
|
||||
|
||||
bool Browser::IsDefaultProtocolClient(const std::string& protocol,
|
||||
gin::Arguments* args) {
|
||||
if (protocol.empty())
|
||||
if (!IsValidProtocolScheme(protocol))
|
||||
return false;
|
||||
|
||||
std::wstring exe;
|
||||
|
||||
@@ -1470,6 +1470,29 @@ describe('app module', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('protocol scheme validation', () => {
|
||||
it('rejects empty protocol names', () => {
|
||||
expect(app.setAsDefaultProtocolClient('')).to.equal(false);
|
||||
expect(app.isDefaultProtocolClient('')).to.equal(false);
|
||||
expect(app.removeAsDefaultProtocolClient('')).to.equal(false);
|
||||
});
|
||||
|
||||
it('rejects non-conformant protocol names ', () => {
|
||||
// Starting with a digit.
|
||||
expect(app.setAsDefaultProtocolClient('0badscheme')).to.equal(false);
|
||||
// Starting with a hyphen.
|
||||
expect(app.setAsDefaultProtocolClient('-badscheme')).to.equal(false);
|
||||
// Containing backslashes.
|
||||
expect(app.setAsDefaultProtocolClient('http\\shell\\open\\command')).to.equal(false);
|
||||
// Containing forward slashes.
|
||||
expect(app.setAsDefaultProtocolClient('bad/protocol')).to.equal(false);
|
||||
// Containing spaces.
|
||||
expect(app.setAsDefaultProtocolClient('bad protocol')).to.equal(false);
|
||||
// Containing colons.
|
||||
expect(app.setAsDefaultProtocolClient('bad:protocol')).to.equal(false);
|
||||
});
|
||||
});
|
||||
|
||||
ifdescribe(process.platform === 'win32')('app launch through uri', () => {
|
||||
it('does not launch for argument following a URL', async () => {
|
||||
const appPath = path.join(fixturesPath, 'api', 'quit-app');
|
||||
|
||||
Reference in New Issue
Block a user