sindresorhus/eslint-plugin-unicorn

Rule proposal: `.replace` or `.replaceAll` with non-literal replacement

Closed

#2,309 opened on Apr 5, 2024

 (3 comments) (4 reactions) (0 assignees)JavaScript (468 forks)user submission
help wantednew rule

Repository metrics

Stars
 (5,022 stars)
PR merge metrics
 (Avg merge 1d 16h) (399 merged PRs in 30d)

Description

Description

One might think that a function like this generates safe HTML because the argument is HTML-escaped.

function imageLink(url) {
  const IMAGE_TEMPLATE = '<img src="{url}">';
  return IMAGE_TEMPLATE.replace('{url}', htmlEscape(url));
}

But in fact there’s a very obscure cross-site scripting vulnerability here, abusing the $` replacement sequence interpreted by String.prototype.replace and .replaceAll!

imageLink("$` onerror=alert(1) ")
//=> '<img src="<img src=" onerror=alert(1) ">'

To protect against this mistake, it would be nice to have an ESLint rule that forbids use of .replace and .replaceAll where the second argument isn’t a string literal or a function.

Fail

IMAGE_TEMPLATE.replace('{url}', htmlEscape(url))

Pass

IMAGE_TEMPLATE.replace('{url}', () => htmlEscape(url))
IMAGE_TEMPLATE.replace('{url}', "https://example.com")

Additional Info

No response

Contributor guide