Skip to content

#6269 improve img srcset sanitation checks - #6270

Open
martgil wants to merge 1 commit into
masterfrom
issue-6269-improve-img-srcset-sanitation
Open

#6269 improve img srcset sanitation checks#6270
martgil wants to merge 1 commit into
masterfrom
issue-6269-improve-img-srcset-sanitation

Conversation

@martgil

@martgil martgil commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

This PR improves img srcset sanitation checks on remote images loaded in content script.

close #6269


Tests (delete all except exactly one):

  • Tests added or updated

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 August 5, 2026 12:25
@martgil
martgil marked this pull request as draft August 5, 2026 12:25
@martgil
martgil marked this pull request as ready for review August 7, 2026 10:08
@martgil

martgil commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

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

if (node.tagName === 'IMG') {
const img = node as HTMLImageElement; // Narrow type to HTMLImageElement
const src = img.getAttribute('src');
const srcset = img.getAttribute('srcset');

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.

I think better solution will be to ignore srcset property by removing it before parsing with img.removeAttribute('srcset');, since basic src support is enough for loading images and srcset just adds support for responsive images. Gmail doesn't support srcset property too, as noted at https://www.caniemail.com/features/html-srcset/

However, we should apply similar URL check for src property, since attacker can use the same /Logout link there too.

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.

Sanitizer should also check srcset when handling remote images

2 participants