Skip to content

Commit

Permalink
fix(security): add dompurify for raw document string pre-cleaning on …
Browse files Browse the repository at this point in the history
…readability to avoid xss attack (#2000)
  • Loading branch information
PrinOrange authored Dec 4, 2024
1 parent 8de8501 commit f2ed678
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 4 deletions.
1 change: 1 addition & 0 deletions apps/main/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"@openpanel/web": "1.0.1",
"@sentry/electron": "5.7.0",
"builder-util-runtime": "9.2.10",
"dompurify": "~3.2.2",
"electron-context-menu": "4.0.4",
"electron-log": "5.2.3",
"electron-squirrel-startup": "1.0.1",
Expand Down
8 changes: 6 additions & 2 deletions apps/main/src/lib/readability.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Readability } from "@mozilla/readability"
import { name, version } from "@pkg"
import DOMPurify from "dompurify"
import { parseHTML } from "linkedom"
import { fetch } from "ofetch"

Expand All @@ -8,7 +9,7 @@ import { isDev } from "~/env"
const userAgents = `Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/127.0.0.0 Safari/537.36 ${name}/${version}`

export async function readability(url: string) {
const documentString = await fetch(url, {
const dirtyDocumentString = await fetch(url, {
headers: {
"User-Agent": userAgents,
Accept: "text/html",
Expand All @@ -26,11 +27,14 @@ export async function readability(url: string) {
return res.text()
})

// For avoid xss attack from readability, the raw document string should be purified.
const cleanedDocumentString = DOMPurify.sanitize(dirtyDocumentString)

// FIXME: linkedom does not handle relative addresses in strings. Refer to
// @see https://github.com/WebReflection/linkedom/issues/153
// JSDOM handles it correctly, but JSDOM introduces canvas binding.

const { document } = parseHTML(documentString)
const { document } = parseHTML(cleanedDocumentString)
const baseUrl = new URL(url).origin

document.querySelectorAll("a").forEach((a) => {
Expand Down
39 changes: 37 additions & 2 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f2ed678

Please sign in to comment.