Skip to content

#6233 Improve css validation#6234

Open
martgil wants to merge 6 commits into
masterfrom
issue-6233-improve-css-validation
Open

#6233 Improve css validation#6234
martgil wants to merge 6 commits into
masterfrom
issue-6233-improve-css-validation

Conversation

@martgil
Copy link
Copy Markdown
Collaborator

@martgil martgil commented May 29, 2026

This PR improves currently implemented css validation on rendered elements in pgp_block.

Closes #6233


Tests (delete all except exactly one):

  • Does not need tests (refactor only, docs or internal changes)

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@martgil martgil requested a review from sosnovsky as a code owner May 29, 2026 08:15
@martgil
Copy link
Copy Markdown
Collaborator Author

martgil commented May 29, 2026

Hi @sosnovsky - This one is ready for a review. Thank you!

Comment thread extension/js/common/platform/xss.ts Outdated
@martgil martgil marked this pull request as draft May 29, 2026 10:30
@martgil martgil marked this pull request as ready for review June 2, 2026 09:01
@martgil martgil requested a review from sosnovsky June 2, 2026 09:02
@martgil

This comment was marked as outdated.

Comment thread extension/js/common/platform/xss.ts
@martgil martgil requested a review from sosnovsky June 3, 2026 11:18
@martgil
Copy link
Copy Markdown
Collaborator Author

martgil commented Jun 3, 2026

Hi @sosnovsky - This one is ready for review. Thank you!

private static HREF_REGEX_CACHE: RegExp | undefined;
private static FORBID_CSS_STYLE = /z-index:[^;]+;|position:[^;]+;|background[^;]+;/g;
private static FORBID_CSS_STYLE =
/(?:^|;)\s*(?:z-index|position|display|visibility|opacity|transform|clip-path|clip|top|left|right|bottom|pointer-events|font-size|line-height|width|height|text-indent|filter)\s*:[^;]*;?/gi;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks like this regex fully removes only first statement, and for next ones it removes just ;:

"position:absolute;z-index:999999;top:0;left:0;width:100%;height:100%;background:white".replace(/(?:^|;)\s*(?:z-index|position|display|visibility|opacity|transform|clip-path|clip|top|left|right|bottom|pointer-events|font-size|line-height|width|height|text-indent|filter)\s*:[^;]*;?/gi, ''); // returns "z-index:999999left:0height:100%;background:white"

fixed regex should look like /(^|;)\s*(?:z-index|position|display|visibility|opacity|transform|clip-path|clip|top|left|right|bottom|pointer-events|font-size|line-height|width|height|text-indent|filter)\s*:[^;]*(?=;|$)/gi

and let's also add test for such cases, to be sure that our implementation works correctly, thanks!

Comment on lines +286 to +298
const urlRegex = /url\(\s*(["']?)(.*?)\1\s*\)/gi;
let match;
// eslint-disable-next-line no-null/no-null
while ((match = urlRegex.exec(cleaned)) !== null) {
const fullMatch = match[0];
const url = match[2];
// Only allow data: and cid: schemes
const isSafe = /^(data:|cid:)/i.test(url);
if (!isSafe) {
// Remove the unsafe url(...) token completely
cleaned = cleaned.replace(fullMatch, '');
}
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

let's replace this while loop with a simpler single replace statement:

cleaned = cleaned.replace(/url\(\s*(["']?)(.*?)\1\s*\)/gi, (fullMatch: string, _, url: string) => {
      // Only allow data: and cid: schemes
      const isSafe = /^(data:|cid:)/i.test(url);
      return isSafe ? fullMatch : '';
    });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve CSS validation

2 participants