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

Switch to headless mode, implement dynamic list selection #26

Merged
merged 3 commits into from
Aug 3, 2024

Conversation

mschfh
Copy link
Collaborator

@mschfh mschfh commented Jul 26, 2024

@mschfh mschfh self-assigned this Jul 26, 2024
@mschfh mschfh force-pushed the mschfh-dynamic-lists branch from 7c46acc to 45ed478 Compare August 1, 2024 01:19
@mschfh mschfh requested a review from antonok-edm August 1, 2024 10:06
@mschfh mschfh force-pushed the mschfh-dynamic-lists branch from d7e2a7b to 4ba9963 Compare August 1, 2024 10:07
@mschfh mschfh changed the title [WIP] Improve adblock list handling Switch to headless mode, implement dynamic list selection Aug 1, 2024
@mschfh mschfh marked this pull request as ready for review August 1, 2024 10:07
@mschfh mschfh force-pushed the mschfh-dynamic-lists branch from 4ba9963 to 9d252e7 Compare August 1, 2024 10:23
// check if valid component id and not in keeplist
if (isValidChromeComponentId({ id: fileName }) && !isKeeplistedComponentId({ id: fileName, additionalComponentList: adblockComponents })) {
console.log('patching component: ', fileName)
const extensionDir = path.join(tmpProfile, fileName)
Copy link

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal


Cc @thypon @bcaller

console.log('patching component: ', fileName)
const extensionDir = path.join(tmpProfile, fileName)
const versionDir = getExtensionVersion(extensionDir)
const srcManifest = path.join(extensionDir, versionDir, 'manifest.json')
Copy link

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal


Cc @thypon @bcaller

const extensionDir = path.join(tmpProfile, fileName)
const versionDir = getExtensionVersion(extensionDir)
const srcManifest = path.join(extensionDir, versionDir, 'manifest.json')
const destPath = path.join(templateProfile, fileName, '999.999')
Copy link

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal


Cc @thypon @bcaller

mkdirSync(destPath, { recursive: true })
if (fileName === 'gkboaolpopklhgplhaaiboijnklogmbc') {
// copy List Catalog files after modifying version
cpSync(path.join(extensionDir, versionDir), destPath, { recursive: true })
Copy link

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal


Cc @thypon @bcaller

cpSync(path.join(extensionDir, versionDir), destPath, { recursive: true })
} else {
// only copy the manifest for all other components
cpSync(srcManifest, path.join(destPath, 'manifest.json'))
Copy link

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal


Cc @thypon @bcaller

}

export const parseListCatalogComponentIds = ({ profileDir }) => {
const extensionDir = path.join(profileDir, 'gkboaolpopklhgplhaaiboijnklogmbc')
Copy link

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal


Cc @thypon @bcaller

const extensionDir = path.join(profileDir, 'gkboaolpopklhgplhaaiboijnklogmbc')
const versionDir = getExtensionVersion(extensionDir)
// gkboaolpopklhgplhaaiboijnklogmbc/1.0.60/list_catalog.json
const data = readFileSync(path.join(extensionDir, versionDir, 'list_catalog.json'), 'utf8')
Copy link

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal


Cc @thypon @bcaller

}

export const getAdblockUuids = ({ profileDir }) => {
const extensionDir = path.join(profileDir, 'gkboaolpopklhgplhaaiboijnklogmbc')
Copy link

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal


Cc @thypon @bcaller

const extensionDir = path.join(profileDir, 'gkboaolpopklhgplhaaiboijnklogmbc')
const versionDir = getExtensionVersion(extensionDir)
// gkboaolpopklhgplhaaiboijnklogmbc/1.0.60/list_catalog.json
const data = readFileSync(path.join(extensionDir, versionDir, 'list_catalog.json'), 'utf8')
Copy link

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal


Cc @thypon @bcaller

listCatalog.forEach(item => {
if (item.uuid in adblockLists) {
item.default_enabled = Boolean(adblockLists[item.uuid])
}
Copy link

Choose a reason for hiding this comment

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

reported by reviewdog 🐶
[semgrep] This allows item.uuid to be __proto__ or toString. To prevent prototype access use Object.prototype.hasOwnProperty.call(adblockLists, item.uuid) or use a Map.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/services/not-using-hasownproperty-proto-access.yaml


Cc @thypon @bcaller

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

koa bodyparser uses co-body, which throws an error by default if a object contains __proto__:

onProtoPoisoning Defines what action the co-body lib must take when parsing a JSON object with __proto__. This functionality is provided by [bourne](https://github.com/hapijs/bourne). See [Prototype-Poisoning](https://fastify.dev/docs/latest/Guides/Prototype-Poisoning/) for more details about prototype poisoning attacks. Possible values are 'error', 'remove' and 'ignore'. Default to 'error', it will throw a SyntaxError when Prototype-Poisoning happen.

Dockerfile Show resolved Hide resolved
Comment on lines +126 to +134
const buttons = await page.$$('.button-check-update')
for (const button of buttons) {
const buttonId = await page.evaluate(el => el.getAttribute('id'), button)
if (buttonId) {
console.log('Updating component:', buttonId)
await button.click()
await setTimeout(50) // Wait for 50ms between clicks
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

there is a Update lists button in brave://settings/shields/filters` which will do the same with 1 click, btw.

src/page.html Outdated Show resolved Hide resolved
src/page.html Outdated Show resolved Hide resolved
src/util.mjs Outdated Show resolved Hide resolved
@antonok-edm
Copy link
Collaborator

also, how is adblock_lists.json intended to be populated? should we fetch it in npm run setup?

@mschfh
Copy link
Collaborator Author

mschfh commented Aug 1, 2024

also, how is adblock_lists.json intended to be populated? should we fetch it in npm run setup?

yes, the content is extracted from list_catalog.json during setup:
https://github.com/brave/cookiemonster/pull/26/files#diff-925fe95ca95b8e7cddc6826a2803dfc14ed9e1654299dec29b4c78e203dfdf25R174-R175

@mschfh mschfh requested a review from antonok-edm August 2, 2024 01:30
@mschfh mschfh merged commit bfe2c70 into main Aug 3, 2024
5 checks passed
@mschfh mschfh deleted the mschfh-dynamic-lists branch August 3, 2024 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants