Skip to content

Commit

Permalink
Remove unnecessary meta element lookups.
Browse files Browse the repository at this point in the history
  • Loading branch information
@sebastian9er
sebastian9er committed Dec 19, 2024
1 parent 5d86c36 commit 1717db0
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 153 deletions.
49 changes: 17 additions & 32 deletions core/audits/clickjacking-mitigation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,52 +33,37 @@ class ClickjackingMitigation extends Audit {
scoreDisplayMode: Audit.SCORING_MODES.INFORMATIVE,
title: str_(UIStrings.title),
description: str_(UIStrings.description),
requiredArtifacts: ['devtoolsLogs', 'MetaElements', 'URL'],
requiredArtifacts: ['devtoolsLogs', 'URL'],
supportedModes: ['navigation'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<{cspHeadersAndMetaTags: string, xfoHeaders: string[]}>}
* @return {Promise<{cspHeaders: string[], xfoHeaders: string[]}>}
*/
static async getRawCspsAndXfo(artifacts, context) {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const mainResource =
await MainResource.request({devtoolsLog, URL: artifacts.URL}, context);
let cspMetaTags = [''];

const cspHeaders =
mainResource.responseHeaders
.filter(h => {
return h.name.toLowerCase() === 'content-security-policy';
})
.flatMap(h => h.value.split(','));

const cspHeaders = mainResource.responseHeaders
.filter(h => {
return h.name.toLowerCase() === 'content-security-policy';
})
.flatMap(h => h.value.split(','))
.filter(rawCsp => rawCsp.replace(/\s/g, ''));
let xfoHeaders = mainResource.responseHeaders
.filter(h => {
return h.name.toLowerCase() === 'x-frame-options';
})
.flatMap(h => h.value);
if (undefined !== artifacts.MetaElements) {
cspMetaTags =
artifacts.MetaElements
.filter(m => {
return m.httpEquiv &&
m.httpEquiv.toLowerCase() === 'content-security-policy';
})
.flatMap(m => (m.content || '').split(','));
}

const cspHeadersAndMetaTags =
cspHeaders.map(v => v.toLowerCase())
.concat(cspMetaTags.map(v => v.toLowerCase()))
.join(';').replace(/\s/g, '');

// Sanitize the XFO header value.
xfoHeaders = xfoHeaders.map(v => v.toLowerCase().replace(/\s/g, ''));

return {cspHeadersAndMetaTags, xfoHeaders};
return {cspHeaders, xfoHeaders};
}

/**
Expand All @@ -96,16 +81,16 @@ class ClickjackingMitigation extends Audit {
}

/**
* @param {string} cspHeadersAndMetaTags
* @param {string[]} cspHeaders
* @param {string[]} xfoHeaders
* @return {{score: number, results: LH.Audit.Details.TableItem[]}}
*/
static constructResults(cspHeadersAndMetaTags, xfoHeaders) {
static constructResults(cspHeaders, xfoHeaders) {
const rawXfo = [...xfoHeaders];
const allowedDirectives = ['deny', 'sameorigin'];

// If there is none of the two headers, return early.
if (!rawXfo.length && !cspHeadersAndMetaTags.length) {
if (!rawXfo.length && !cspHeaders.length) {
return {
score: 0,
results: [{
Expand All @@ -117,8 +102,8 @@ class ClickjackingMitigation extends Audit {
}

// Check for frame-ancestors in CSP.
if (cspHeadersAndMetaTags.length) {
for (const cspDirective of cspHeadersAndMetaTags.split(';')) {
if (cspHeaders.length) {
for (const cspDirective of cspHeaders) {
if (cspDirective.includes('frame-ancestors')) {
// Pass the audit if frame-ancestors is present.
return {score: 1, results: []};
Expand Down Expand Up @@ -149,8 +134,8 @@ class ClickjackingMitigation extends Audit {
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts, context) {
const {cspHeadersAndMetaTags, xfoHeaders} = await this.getRawCspsAndXfo(artifacts, context);
const {score, results} = this.constructResults(cspHeadersAndMetaTags, xfoHeaders);
const {cspHeaders, xfoHeaders} = await this.getRawCspsAndXfo(artifacts, context);
const {score, results} = this.constructResults(cspHeaders, xfoHeaders);

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
Expand Down
129 changes: 8 additions & 121 deletions core/test/audits/clickjacking-mitigation-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ it('marked N/A if no violations found', async () => {
mainDocumentUrl: 'https://example.com',
finalDisplayedUrl: 'https://example.com',
},
MetaElements: [],
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog([
{
Expand All @@ -39,7 +38,6 @@ it('marked N/A if no violations found', async () => {

it('No XFO header but CSP header found', async () => {
const artifacts = {
MetaElements: [],
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog([
{
Expand Down Expand Up @@ -69,7 +67,6 @@ it('No XFO header but CSP header found', async () => {

it('No CSP header but XFO header found', async () => {
const artifacts = {
MetaElements: [],
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog([
{
Expand Down Expand Up @@ -98,39 +95,6 @@ it('No CSP header but XFO header found', async () => {
});

it('No CSP and no XFO headers but foo header found', async () => {
const artifacts = {
MetaElements: [],
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog([
{
url: 'https://example.com',
responseHeaders: [
{name: 'Foo-Header', value: `same-origin`},
],
},
]),
},
URL: {
requestedUrl: 'https://example.com',
mainDocumentUrl: 'https://example.com',
finalDisplayedUrl: 'https://example.com',
},
};

const results =
await ClickjackingMitigation.audit(artifacts, {computedCache: new Map()});
expect(results.notApplicable).toBeFalsy();
expect(results.details.items[0].severity).toBeDisplayString('High');
expect(results.details.items[0].description)
.toBeDisplayString('No Clickjacking mitigation found.');
expect(results.details.items).toMatchObject([
{
directive: undefined,
},
]);
});

it('No CSP and no XFO headers and not Meta but foo header found', async () => {
const artifacts = {
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog([
Expand Down Expand Up @@ -164,7 +128,6 @@ it('No CSP and no XFO headers and not Meta but foo header found', async () => {

it('Messed up XFO directive and no CSP present.', async () => {
const artifacts = {
MetaElements: [],
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog([
{
Expand Down Expand Up @@ -197,7 +160,6 @@ it('Messed up XFO directive and no CSP present.', async () => {

it('Messed up CSP directive and no XFO present.', async () => {
const artifacts = {
MetaElements: [],
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog([
{
Expand Down Expand Up @@ -230,7 +192,6 @@ it('Messed up CSP directive and no XFO present.', async () => {

it('Messed up CSP and XFO directives.', async () => {
const artifacts = {
MetaElements: [],
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog([
{
Expand Down Expand Up @@ -262,81 +223,9 @@ it('Messed up CSP and XFO directives.', async () => {
]);
});

it('Frame-ancestors in the MetaElement and no XFO present.', async () => {
const artifacts = {
MetaElements: [
{
httpEquiv: 'Content-Security-Policy',
content: `frame-ancestors 'none'`,
},
],
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog([
{
url: 'https://example.com',
responseHeaders: [
],
},
]),
},
URL: {
requestedUrl: 'https://example.com',
mainDocumentUrl: 'https://example.com',
finalDisplayedUrl: 'https://example.com',
},
};

const results =
await ClickjackingMitigation.audit(artifacts, {computedCache: new Map()});
expect(results.details.items).toHaveLength(0);
expect(results.notApplicable).toBeTruthy();
});

it('Messed up CSP directive in the header, but frame-ancestors in the MetaElement and no XFO present.',
async () => {
const artifacts = {
MetaElements: [
{
httpEquiv: 'Content-Security-Policy',
content: `frame-ancestors 'self'`,
},
],
devtoolsLogs: {
defaultPass: networkRecordsToDevtoolsLog([
{
url: 'https://example.com',
responseHeaders: [
{
name: 'Content-Security-Policy',
value:
`foo-directive 'none'; report-uri https://csp.example.com`
},
],
},
]),
},
URL: {
requestedUrl: 'https://example.com',
mainDocumentUrl: 'https://example.com',
finalDisplayedUrl: 'https://example.com',
},
};

const results =
await ClickjackingMitigation.audit(artifacts, {computedCache: new Map()});
expect(results.details.items).toHaveLength(0);
expect(results.notApplicable).toBeTruthy();
});

describe('getRawCspsAndXfo', () => {
it('basic case', async () => {
const artifacts = {
MetaElements: [
{
httpEquiv: 'Content-Security-Policy',
content: `default-src 'none'`,
},
],
URL: {
requestedUrl: 'https://example.com',
mainDocumentUrl: 'https://example.com',
Expand All @@ -360,16 +249,15 @@ describe('getRawCspsAndXfo', () => {
]),
},
};
const {cspHeadersAndMetaTags, xfoHeaders} =
const {cspHeaders, xfoHeaders} =
await ClickjackingMitigation.getRawCspsAndXfo(artifacts, {computedCache: new Map()});
expect(cspHeadersAndMetaTags).toEqual(`frame-ancestors'self';default-src'none'`);
expect(cspHeaders).toEqual(["frame-ancestors 'self'"]);
expect(xfoHeaders).toEqual([`sameorigin`]);
});

it('ignore if empty', async () => {
const artifacts = {
MetaElements: [],
URL: {
URL: {
requestedUrl: 'https://example.com',
mainDocumentUrl: 'https://example.com',
finalDisplayedUrl: 'https://example.com',
Expand All @@ -392,18 +280,17 @@ describe('getRawCspsAndXfo', () => {
]),
},
};
const {cspHeadersAndMetaTags, xfoHeaders} =
const {cspHeaders, xfoHeaders} =
await ClickjackingMitigation.getRawCspsAndXfo(artifacts, {computedCache: new Map()});
expect(cspHeadersAndMetaTags).toEqual(``);
expect(cspHeaders).toEqual([]);
expect(xfoHeaders).toEqual([
`deny`,
]);
});

it('ignore if only whitespace', async () => {
const artifacts = {
MetaElements: [],
URL: {
URL: {
requestedUrl: 'https://example.com',
mainDocumentUrl: 'https://example.com',
finalDisplayedUrl: 'https://example.com',
Expand All @@ -426,9 +313,9 @@ describe('getRawCspsAndXfo', () => {
]),
},
};
const {cspHeadersAndMetaTags, xfoHeaders} =
const {cspHeaders, xfoHeaders} =
await ClickjackingMitigation.getRawCspsAndXfo(artifacts, {computedCache: new Map()});
expect(cspHeadersAndMetaTags).toEqual(``);
expect(cspHeaders).toEqual([]);
expect(xfoHeaders).toEqual([
`deny`,
]);
Expand Down

0 comments on commit 1717db0

Please sign in to comment.