Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-enable CSP via whitelist #1662

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
99 changes: 95 additions & 4 deletions injector/src/modules/csp.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,105 @@
import electron from "electron";

const whitelist = {
// Discord includes unsafe-inline already
script: [
"https://*.github.io",
Copy link
Member

@Inve1951 Inve1951 Sep 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think *.github.io can be removed. If some plugin still relies on it, it shall break.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

"https://cdnjs.cloudflare.com" // Used for Monaco
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of hosting Monaco on betterdiscord.app instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open to tossing around the idea, but with our site issues over the last several months I'm hesitant.

],

// Discord includes unsafe-inline already
style: [
"https://*.github.io",
"https://cdnjs.cloudflare.com", // Used for Monaco
"https://fonts.googleapis.com",
"https://fonts.cdnfonts.com",
"https://cdn.statically.io",
"https://rawgit.com",
"https://raw.githack.com",
"https://rsms.me",
"https://cdn.jsdelivr.net",
],

// Discord includes fonts.gstatic.com
font: [
"data:",
"https://*.github.io",
"https://cdnjs.cloudflare.com",
"https://fonts.googleapis.com",
"https://raw.githack.com",
"https://cdn.jsdelivr.net",
],

// Discord includes several sources already including imgur and data:
img: [
"https://*.github.io",
"https://ik.imagekit.io",
"https://source.unsplash.com",
"https://raw.githubusercontent.com",
"https://svgur.com",
"https://i.ibb.co",
"https://rawgit.com",
"https://bowmanfox.xyz",
"https://paz.pw",
"https://adx74.fr",
"https://media.tenor.com", // included by discord already
"https://upload.wikimedia.org",
"https://svgrepo.com",
"https://ch3rry.red",
"https://teamcofh.com",
"https://icon-library.net",
"https://images.pexels.com",
"https://user-images.githubusercontent.com",
"https://emoji.gg",
"https://cdn-icons-png.flaticon.com",
],

// Discord does not include this normally
worker: [
"'self'", // To allow Discord's own workers
"data:", // Used for Monaco
],

};

const types = Object.keys(whitelist);

const strings = {};
for (const key of types) strings[key] = whitelist[key].join(" ") + " ";


function addToCSP(csp, type) {
// If it's in the middle of the policy (most common case)
if (csp.includes(`; ${type}-src `)) return csp.replace(`; ${type}-src `, `; ${type}-src ${strings[type]}`);

// If it's the first rule (very uncommon, default-src should be first)
else if (csp.includes(`${type}-src `)) return csp.replace(`${type}-src `, `${type}-src ${strings[type]}`);

// Otherwise, rule doesn't exist, add it to the end
return csp = csp + `; ${type}-src ${strings[type]}; `;
}

export default class {
static remove() {
electron.session.defaultSession.webRequest.onHeadersReceived(function(details, callback) {
const headers = Object.keys(details.responseHeaders);
for (let h = 0; h < headers.length; h++) {
const key = headers[h];
const headerKeys = Object.keys(details.responseHeaders);
for (let h = 0; h < headerKeys.length; h++) {
const key = headerKeys[h];

// Because the casing is inconsistent for whatever reason...
if (key.toLowerCase().indexOf("content-security-policy") !== 0) continue;
delete details.responseHeaders[key];

// Grab current CSP policy and make sure it's Discord's
// since that's the only one we need to modify
let csp = details.responseHeaders[key];
if (Array.isArray(csp)) csp = csp[0];
if (!csp.toLowerCase().includes("discordapp")) continue;

// Iterate over all whitelisted types and update the header
for (let k = 0; k < types.length; k++) csp = addToCSP(csp, types[k]);
details.responseHeaders[key] = [csp];
}

callback({cancel: false, responseHeaders: details.responseHeaders});
});
}
Expand Down
62 changes: 38 additions & 24 deletions renderer/src/modules/updater.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,31 +76,45 @@ export class CoreUpdater {
}

static async checkForUpdate(showNotice = true) {
const resp = await fetch(`https://api.github.com/repos/BetterDiscord/BetterDiscord/releases/latest`,{
method: "GET",
headers: {
"Accept": "application/json",
"Content-Type": "application/json",
"User-Agent": "BetterDiscord Updater"
}
});

const data = await resp.json();
this.apiData = data;
const remoteVersion = data.tag_name.startsWith("v") ? data.tag_name.slice(1) : data.tag_name;
this.hasUpdate = remoteVersion > Config.version;
this.remoteVersion = remoteVersion;
if (!this.hasUpdate || !showNotice) return;
try {
const buffer = await new Promise((resolve, reject) => {
request({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure about using request?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this will definitely be updated

url: "https://api.github.com/repos/BetterDiscord/BetterDiscord/releases/latest",
method: "get",
headers: {
"Accept": "application/json",
"Content-Type": "application/json",
"User-Agent": "BetterDiscord Updater"
}
}, (err, resp, body) => {
if (err || resp.statusCode != 200) return reject(err || `${resp.statusCode} ${resp.statusMessage}`);
return resolve(body);
});
});

const close = Notices.info(Strings.Updater.updateAvailable.format({version: remoteVersion}), {
buttons: [{
label: Strings.Notices.moreInfo,
onClick: () => {
close();
UserSettingsWindow?.open?.("updates");
}
}]
});
const data = JSON.parse(buffer.toString());
this.apiData = data;
const remoteVersion = data.tag_name.startsWith("v") ? data.tag_name.slice(1) : data.tag_name;
this.hasUpdate = remoteVersion > Config.version;
this.remoteVersion = remoteVersion;
if (!this.hasUpdate || !showNotice) return;

const close = Notices.info(Strings.Updater.updateAvailable.format({version: remoteVersion}), {
buttons: [{
label: Strings.Notices.moreInfo,
onClick: () => {
close();
UserSettingsWindow?.open?.("updates");
}
}]
});
}
catch (err) {
Logger.stacktrace("Updater", "Failed to check update", err);
Modals.showConfirmationModal(Strings.Updater.updateFailed, Strings.Updater.updateFailedMessage, {
cancelText: null
});
}
}

static async update() {
Expand Down