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

[andersk]

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

[sindresorhus]

What should happen to IMAGE_TEMPLATE.replace('{url}', foo) if we cannot infer to it be a string literal?

[andersk]

It would be disallowed.

If foo is really a string, I’d expect most people who write an invocation like that are expecting the behavior of IMAGE_TEMPLATE.replace('{url}', () => foo). If it’s really a function, the invocation can be rewritten IMAGE_TEMPLATE.replace('{url}', (...match) => foo(...match)) (a la no-array-callback-reference).

[sindresorhus]

Accepted