Fix CVE-2022-4993 stop routing foreign text into the Locale::Maketext format by robrwo · Pull Request #159 · gshank/html-formhandler

robrwo · 2026-08-14T08:51:39Z

The first argument to add_error is the Locale::Maketext FORMAT: bracket groups in it are compiled into method-dispatch code. Three kinds of text that FormHandler did not author reach that position, all of them carrying submitted request data:

_apply_actions traps warnings into $error_message (Validate.pm, the $SIG{WARN} handler). A warning survives a SUCCESSFUL action, so an ordinary numeric transform on a text field turns Argument "[sprintf,%2000000000d,0]" isn't numeric into the format -- Perl quotes the value verbatim, so the group is well formed and reaches CORE::sprintf with an attacker-chosen width (~GB allocation).
a type constraint's failure message. Moose renders the rejected value with Devel::PartialDump when it can load it, Type::Tiny always uses its own dumper, and both render a reference in bracket-and-comma form -- so on any field with apply => [ Str ], two same-named request parameters put [ "a", "b" ] in the format and maketext croaks, which add_error re-dies: an unhandled 500 with no payload at all.
exceptions from a coercion or transform.
a date parser's error message. Field/Date.pm passes $strp->errmsg || $@ -- DateTime::Format::Strptime's own text -- straight into the format position. DTFS 1.80 answers a rejected value with the fixed string "Your datetime does not match your pattern.", so there is no reachable payload through it today; that is a property of the current version of a separate distribution rather than of this code, and the || $@ fallback is a second channel that was not exercised. Escaped for the same reason as the others.

Escape the bracket-notation metacharacters in all four before they are used as a format. Tilde is Locale::Maketext's escape, and text with no brackets is returned unchanged, so lexicon lookups and translated type-constraint messages are byte-identical to before.

Also: add_error derefs an arrayref first argument into (template, @Args). That spelling is the same list-or-arrayref convenience idiom as add_element_class and friends; it is not documented for add_error, and an instrumented run of the distribution's own suite (150 files, 1491 tests) never reaches the branch. What does reach it is request data -- $field->add_error($field->value) where the request parser folded a duplicate parameter into an arrayref puts submitted text in element 0. Since no message the library raises arrives in that shape, treat an arrayref argument as a value: keep the deref, render element 0 literally. A caller who wants a compiled template passes it as a plain list, $field->add_error($template, @args), which is the documented spelling and is unchanged.

One case cannot be fixed here: an application that concatenates the value into its own message, add_error("The value '" . $field->value . "' is not allowed"), is indistinguishable from a legitimate template, so the add_error POD now documents the hazard and the inert-argument idiom.

Behaviour trade-offs -- the only output changes outside the attack cases:

a custom type constraint whose own message block uses bracket notation (message { 'Try [quant,1,thing]' }) now renders that literally. Such a block receives the rejected value and can interpolate it, so escaping it is the safe default; a maintainer who would rather keep those compiled can exempt the has_message branch specifically.
an application calling the undocumented arrayref spelling with a template that uses bracket notation, add_error([ 'Try [quant,_1,thing]', 3 ]), now renders it literally; the list spelling of the same call still compiles.

FormHandler's own message templates, and application templates passed to add_error together with their arguments, are unaffected.

Verified against the 0.40068 test suite: 150 files, 1491 tests, PASS both before and after. A before/after table of rendered error messages (maxlength, minlength, required, invalid select value, integer range, duplicate-parameter arrays, application template with arguments, plain and bracketed type messages, plain and bracketed warnings and exceptions) is byte-identical except the lines above.

robrwo · 2026-08-14T12:47:50Z

@abraxxa @zby For future reference, who is maintaining this module, and who should the security contact be?

abraxxa

We should probably link to https://metacpan.org/dist/Locale-Maketext/view/lib/Locale/Maketext.pod#BRACKET-NOTATION-SECURITY in the pod.

Can we define an (empty? minimal?) allowlist to improve security more than escaping unknown input?

abraxxa · 2026-08-17T07:49:07Z


+# Locale::Maketext treats its FORMAT argument as bracket-notation source: any
+# '[...]' group inside it is compiled into method-dispatch code (see _compile
+# in Locale::Maketext). Messages FormHandler itself authors are templates on


I don't understand this sentence, is it a typo?

No, it makes sense to me.

The format argument is assumed to be bracket-notation-template and compiled.

I was referring to Messages FormHandler itself authors are templates on purpose, but three kinds of text reaching _apply_actions are not ours and do embed request data:.

I read it (after a few goes) as "The messages that FormHandler itself authors are (deliberately) templates, but there are three kinds of text that reach _apply_actions that can end up embedding request data in some way:"

robrwo · 2026-08-17T08:53:56Z

We should probably link to https://metacpan.org/dist/Locale-Maketext/view/lib/Locale/Maketext.pod#BRACKET-NOTATION-SECURITY in the pod.

Can we define an (empty? minimal?) allowlist to improve security more than escaping unknown input?

We can try that and see if it works. I'm unsure part of the problem is that it may not work as well as it should.

abraxxa

We could also just pass the error message always as second argument and '[_1]' as first one, which wouldn't require escaping the error messages.

robrwo · 2026-08-17T08:57:36Z

Keep in mind that the code was mostly written by Claude. So there is likely a much better way of doing this.

abraxxa · 2026-08-17T09:00:59Z

Can you please add tests that prove to be not vulnerable? Thanks!

@Args

…t format The first argument to add_error is the Locale::Maketext FORMAT: bracket groups in it are compiled into method-dispatch code. Three kinds of text that FormHandler did not author reach that position, all of them carrying submitted request data: * _apply_actions traps warnings into $error_message (Validate.pm, the $SIG{__WARN__} handler). A warning survives a SUCCESSFUL action, so an ordinary numeric transform on a text field turns `Argument "[sprintf,%2000000000d,0]" isn't numeric` into the format -- Perl quotes the value verbatim, so the group is well formed and reaches CORE::sprintf with an attacker-chosen width (~GB allocation). * a type constraint's failure message. Moose renders the rejected value with Devel::PartialDump when it can load it, Type::Tiny always uses its own dumper, and both render a reference in bracket-and-comma form -- so on any field with `apply => [ Str ]`, two same-named request parameters put `[ "a", "b" ]` in the format and maketext croaks, which add_error re-dies: an unhandled 500 with no payload at all. * exceptions from a coercion or transform. * a date parser's error message. Field/Date.pm passes `$strp->errmsg || $@` -- DateTime::Format::Strptime's own text -- straight into the format position. DTFS 1.80 answers a rejected value with the fixed string "Your datetime does not match your pattern.", so there is no reachable payload through it today; that is a property of the current version of a separate distribution rather than of this code, and the `|| $@` fallback is a second channel that was not exercised. Escaped for the same reason as the others. Escape the bracket-notation metacharacters in all four before they are used as a format. Tilde is Locale::Maketext's escape, and text with no brackets is returned unchanged, so lexicon lookups and translated type-constraint messages are byte-identical to before. Escape the bracket-notation metacharacters in all four before they are used as a format. Tilde is Locale::Maketext's escape, and text with no brackets is returned unchanged, so lexicon lookups and translated type-constraint messages are byte-identical to before. Also: add_error derefs an arrayref first argument into (template, @Args). That spelling is the same list-or-arrayref convenience idiom as add_element_class and friends; it is not documented for add_error, and an instrumented run of the distribution's own suite (150 files, 1491 tests) never reaches the branch. What does reach it is request data -- `$field->add_error($field->value)` where the request parser folded a duplicate parameter into an arrayref puts submitted text in element 0. Since no message the library raises arrives in that shape, treat an arrayref argument as a value: keep the deref, render element 0 literally. A caller who wants a compiled template passes it as a plain list, `$field->add_error($template, @Args)`, which is the documented spelling and is unchanged. One case cannot be fixed here: an application that concatenates the value into its own message, `add_error("The value '" . $field->value . "' is not allowed")`, is indistinguishable from a legitimate template, so the add_error POD now documents the hazard and the inert-argument idiom. Behaviour trade-offs -- the only output changes outside the attack cases: * a custom type constraint whose own message block uses bracket notation (message { 'Try [quant,1,thing]' }) now renders that literally. Such a block receives the rejected value and can interpolate it, so escaping it is the safe default; a maintainer who would rather keep those compiled can exempt the has_message branch specifically. * an application calling the undocumented arrayref spelling with a template that uses bracket notation, add_error([ 'Try [quant,_1,thing]', 3 ]), now renders it literally; the list spelling of the same call still compiles. FormHandler's own message templates, and application templates passed to add_error together with their arguments, are unaffected. Verified against the 0.40068 test suite: 150 files, 1491 tests, PASS both before and after. A before/after table of rendered error messages (maxlength, minlength, required, invalid select value, integer range, duplicate-parameter arrays, application template with arguments, plain and bracketed type messages, plain and bracketed warnings and exceptions) is byte-identical except the lines above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Robert Rothenberg <rrwo@cpansec.org>

robrwo · 2026-08-18T08:02:58Z

To be fair, I am unhappy with the possible solutions. I like the idea of passing '[_1]' plus the error better, but caching of values breaks that.

Edit: reading up on this, no, it shouldn't be caching anything because it will always get the same '[_1]` template. Whereas the escaping version still caches values.

So passing the value as the second argument is the better version.

…:Maketext format" This reverts commit c7c69bb.

@Args

…rmat This implements the approach you suggested, after the earlier patch -- which escaped the bracket metacharacters in the text instead -- was declined. Thank you for the suggestion; it is a better fix than the one it replaces, for the reasons set out below, and it is shorter. Generated against the SOURCE REPOSITORY at 0.40068-28-gb032539, and `git apply` takes it cleanly there. It deliberately does NOT apply to the 0.40068 release tarball: the coercion branch of _apply_actions changed after the tag (6f58359, "Check the return value of an eval call"), so the context around one hunk differs between the two trees. The defect itself is present in 0.40068 and every earlier release -- only the fix's anchor point moved. There is no MANIFEST hunk, on purpose: MANIFEST is a Dist::Zilla build product and is not in the repository, so a hunk touching it could not apply -- and `git apply` is all-or-nothing, which would take the code hunks down with it. Adding t/errors/maketext_inert_argument.t to the repository is enough; the [Manifest] plugin picks it up at build time. The problem. The first argument to _localize is the Locale::Maketext FORMAT: a "[...]" group in it is compiled into method-dispatch code. Several kinds of text that FormHandler did not author reach that position, all of them carrying submitted request data: * _apply_actions traps warnings into $error_message (Validate.pm, the $SIG{__WARN__} handler). A warning survives a SUCCESSFUL action, so an ordinary numeric transform on a text field turns `Argument "[sprintf,%2000000000d,0]" isn't numeric` into the format -- Perl quotes the value verbatim, so the group is well formed and reaches CORE::sprintf with a submitter-chosen width (~GB allocation). * a type constraint's failure message. Moose renders the rejected value with Devel::PartialDump when it can load it, Type::Tiny always uses its own dumper, and both render a reference in bracket-and-comma form -- so on any field with `apply => [ Str ]`, two same-named request parameters put `[ "a", "b" ]` in the format and maketext croaks, which add_error re-dies: an unhandled 500 with no payload at all. * exceptions from a coercion or transform. * add_error's arrayref first argument, dereferenced into (template, @Args). That spelling is the same list-or-arrayref convenience idiom as add_element_class and friends; it is not documented for add_error, and an instrumented run of the distribution's own suite never reaches the branch. What does reach it is request data -- `$field->add_error($field->value)` where the request parser folded a duplicate parameter into an arrayref puts submitted text in element 0, and it is the one route that hands over the ARGUMENTS as well as the template. The fix. Hand such text to the localizer as an argument, with a constant format -- the idiom the add_error POD already recommends to applications: $self->add_error( '[_1]', $foreign_text ); A $foreign_message flag records which of the assignments in _apply_actions put text there that this distribution did not write; at the single point where that function calls add_error, such text moves into an argument slot. A message the application supplied through `action => { message => ... }` is a template on purpose and is left alone. Field.pm's add_error does the same for element 0 of an arrayref argument. References count as foreign text, and are diverted too. Every source above can deliver one: `warn $object` hands the object itself to $SIG{__WARN__}, `die $object` in a transform or a coercion leaves it in $@, and a type constraint's message block may return one -- all five confirmed reachable by test. Locale:: Maketext then stringifies whatever it is handed as the format (_compile's own fast-path regex does it before anything else), honouring a '""' overload, so an object stringifying to a bracket group would slip past a bailout on references and be compiled anyway. Testing the stringification instead would not close it: Maketext performs its own, later stringification, so inspecting one and compiling the other leaves a gap, and an overload is arbitrary code that need not answer the same way twice. Handing the object over as an argument sidesteps all of it: an argument is interpolated, never compiled, so it stringifies exactly as it did before a bracket group was ever involved, and only the compilation is gone. The interpolation is written explicitly, because a format consisting of one bracket group compiles to a single chunk and Locale::Maketext only prepends its `join ''` when there is more than one -- so '[_1]' by itself hands back the argument as it was, which would put an exception object into the errors attribute where the type constraint is ArrayRef[Str]. Why this is better than escaping, beyond being shorter. Escaping requires knowing that the escape character is "~", that both brackets need it, and that "~" must be doubled before the brackets are touched; getting exactly that wrong in the other direction is CVE-2012-6329. Passing the text as an argument means it is never parsed as anything, so there is no such rule to get right. And it fixes something escaping does not. With _AUTO set -- the default for HTML::FormHandler::I18N::en_us -- Locale::Maketext memoises every distinct format into a package-global hash that is never evicted and outlives the handle it was compiled for. Using submitted text as the format therefore accumulates a permanent entry per distinct value for as long as the process runs. Escaping the text does not help there; it just memoises the escaped form instead. Measured over five distinct payloads through the warning route: five permanent compiled entries both unpatched and with the escaping patch, one with this patch, however many payloads arrive. Behaviour trade-offs -- the only output changes outside the attack cases: * text this distribution did not author is no longer used as a lexicon key, so a custom type constraint's own message is shown as the type gave it rather than translated. For a deployment using the default _AUTO lexicon this changes nothing, since an _AUTO lookup returns the phrase itself; it matters only where an application has such a message as a real lexicon entry. If you would rather keep that lookup, _needs_inert_format can return false for text containing none of "~", "[" or "]" -- that is the complete set of characters Locale::Maketext's compiler treats specially, so such text cannot form a group and is safe as a format. The comment above the sub says so, and the corresponding assertion in the new test would need inverting. Both variants were measured; this one is the default here because it is the simpler rule and because of the memoisation above. * a custom type constraint whose own message block uses bracket notation (message { 'Try [quant,1,thing]' }) now renders that literally. Such a block receives the rejected value and can interpolate it, so treating it as data is the safe default; a maintainer who would rather keep those compiled can exempt the has_message branch specifically. * an application calling the undocumented arrayref spelling with a template that uses bracket notation, add_error([ 'Try [quant,_1,thing]', 3 ]), now renders it literally; the list spelling of the same call still compiles. FormHandler's own message templates, and application templates passed to add_error together with their arguments, are unaffected. One case cannot be fixed in this distribution: an application that concatenates the value into its own message, `add_error("The value '" . $field->value . "' is not allowed")`, is indistinguishable from a legitimate template. The add_error POD now documents the hazard and shows the inert-argument idiom against it. The new test, t/errors/maketext_inert_argument.t, uses only Test::More and Test::Exception, both already in TEST_REQUIRES. Its first assertions fail without this patch -- the payload is dispatched rather than shown, or processing dies -- and the rest pin behaviour the fix must not break: built-in messages, bracket notation in application templates, and a bracket-free warning still used as the message. Those pass either way, by design. The sprintf assertion deliberately uses a 20-character width rather than a large one: it detects that dispatch happened, without allocating anything. Verified against the repository tree at 0.40068-28-gb032539: the suite is green before (149 files, 1494 tests) and after (150 files, 1511 tests), both touched files compile, and the new test fails on the unpatched tree and passes 17/17 on the patched one. Note for anyone reproducing that: t/fields/request_token.t needs Test::Needs, added by c49c5f6 after the tag -- without it that file dies at compile time with zero tests run, which looks like patch fallout and is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Robert Rothenberg <rrwo@cpansec.org>

This was added in 2011 but smart matchting has been deprecated.

abraxxa requested review from abraxxa and zby August 14, 2026 08:57

abraxxa self-assigned this Aug 14, 2026

abraxxa requested changes Aug 17, 2026

View reviewed changes

abraxxa reviewed Aug 17, 2026

View reviewed changes

robrwo force-pushed the CVE-2022-4993 branch from dbca6c0 to c7c69bb Compare August 17, 2026 16:47

robrwo requested a review from abraxxa August 17, 2026 18:24

Revert "HTML::FormHandler: stop routing foreign text into the Locale:…

bbebaef

…:Maketext format" This reverts commit c7c69bb.

robrwo force-pushed the CVE-2022-4993 branch from 75811ff to 78c7c81 Compare August 21, 2026 09:23

robrwo added 2 commits August 21, 2026 10:49

Remove comment about smart matching

3b00836

This was added in 2011 but smart matchting has been deprecated.

robrwo force-pushed the CVE-2022-4993 branch from 78c7c81 to 3b00836 Compare August 21, 2026 09:50

Navigation Menu

Fix CVE-2022-4993 stop routing foreign text into the Locale::Maketext format - #159

Fix CVE-2022-4993 stop routing foreign text into the Locale::Maketext format#159
robrwo wants to merge 4 commits into
gshank:masterfrom
robrwo:CVE-2022-4993

robrwo commented Aug 14, 2026

Uh oh!

robrwo commented Aug 14, 2026

Uh oh!

abraxxa left a comment

Uh oh!

abraxxa Aug 17, 2026

Uh oh!

robrwo Aug 17, 2026

Uh oh!

abraxxa Aug 17, 2026

Uh oh!

dracos Aug 18, 2026 •

edited

Loading

Uh oh!

robrwo commented Aug 17, 2026

Uh oh!

abraxxa left a comment

Uh oh!

robrwo commented Aug 17, 2026

Uh oh!

abraxxa commented Aug 17, 2026

Uh oh!

robrwo commented Aug 18, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

robrwo commented Aug 14, 2026

Uh oh!

robrwo commented Aug 14, 2026

Uh oh!

abraxxa left a comment

Choose a reason for hiding this comment

Uh oh!

abraxxa Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

robrwo Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

abraxxa Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

dracos Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robrwo commented Aug 17, 2026

Uh oh!

abraxxa left a comment

Choose a reason for hiding this comment

Uh oh!

robrwo commented Aug 17, 2026

Uh oh!

abraxxa commented Aug 17, 2026

Uh oh!

robrwo commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dracos Aug 18, 2026 •

edited

Loading

robrwo commented Aug 18, 2026 •

edited

Loading