A simple invalid 2 octet sequence can cause an exception (which then …

zendframework / zend-escaper

Escaper component from Zend Framework

BSD 3-Clause "New" or "Revised" License

332 stars 30 forks source link

A simple invalid 2 octet sequence can cause an exception (which then … #29

Closed Saeven closed 6 years ago

Saeven commented 6 years ago

A change is necessary to prevent routine penetration tests, or user-land attacks from causing critical exceptions. Users shouldn't be able to craft malicious input that causes 500s when using components such as Zend Form.

Did my best to eyeball formatting conventions, phpcs is in a panic with the given config.

Debug added as second arg to ctor; but could separately be moved into (or complemented by) a setDebug method.

weierophinney commented 6 years ago

@Ocramius For context, please see the related issue in zend-form:

https://github.com/zendframework/zend-form/pull/193

I asked @Saeven to open the issue here, because it is applicable, and there's a generalized issue in escapeHtmlAttr() when malformed UTF-8 is present for the value.

The fix is needed; the question is whether the approach here is correct, or needs revisions.

Ocramius commented 6 years ago

@weierophinney we will bring the issue offline first and discuss it. The original issue is clear, the fix is not valid though, and the default behavior is correct, while this particular patch would only provide a new attack surface.

Invalid values do need to crash systems by default, as otherwise:

bugs go unnoticed
logic execution continues even with invalid output (think legal documents/PDFs with unexpected blank fields, for example, and the chaos that causes later on)

weierophinney commented 6 years ago

@Ocramius Just now catching up on the Slack discussion and zend-form discussion; I reacted hastily.