Skip to content

Fix transformation factory name validation and empty action payload handling #3740

Open

Description

@coderabbitai

Summary

Track three related problems in transformation factory and action payload handling. These are follow-up work outside the scope of PR #3738.

Requested by @airween.

Problems

  1. Unknown names return a non-null no-op transformation. Transformation::instantiate() returns a base Transformation for an unknown name instead of nullptr. Callers that check for a null result do not detect the invalid name. Lua m.getvar() already had this behavior before PR fix: Lua m.getvar()/m.getvars() transformation arguments #3738; the PR makes m.getvars() use the same transformation application path.
  2. IF_MATCH() matches only a name prefix. For example, lowercasefoo can instantiate LowerCase. Before the Lua colon guard in PR fix: Lua m.getvar()/m.getvars() transformation arguments #3738, lowercase: could also reach this path.
  3. Empty payloads cause an out-of-range access. Action::set_name_and_payload() calls m_parser_payload.at(0) when the parsed payload is empty. Handle empty payloads before accessing the first character.

Required changes and affected areas

  • Update src/actions/transformations/transformation.cc and its factory declaration as needed to distinguish unknown names from valid transformations. Preserve transformation as a supported no-op name.
  • Make factory name matching validate the complete transformation name rather than accepting a known-name prefix.
  • Update Action::set_name_and_payload() to handle an empty payload safely and report or reject invalid input through the intended error path.
  • Audit all Transformation::instantiate() consumers before changing the factory contract. Include Lua handling in src/engine/lua.cc, ModSecurity::processContentOffset() in src/modsecurity.cc, and factory tests.
  • Keep the Lua colon-name rejection introduced in PR fix: Lua m.getvar()/m.getvars() transformation arguments #3738. Do not rely on that caller-specific guard as the factory or action parser's only protection.

Acceptance criteria

  • Unknown transformation names return nullptr, or an explicit unmatched result under a documented factory contract.
  • All factory consumers handle the unknown-name result safely.
  • Lua m.getvar() and m.getvars() report unknown transformation names rather than silently treating them as valid no-ops.
  • The exact supported name transformation remains a valid no-op.
  • lowercase still resolves to LowerCase, while lowercasefoo does not.
  • Empty payload input, including the t:lowercase: case, does not cause std::out_of_range from m_parser_payload.at(0).
  • Add regression coverage for unknown names, prefix collisions, the valid no-op name, and empty payload handling. Cover both Lua entry points and affected non-Lua consumers where applicable.

References

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions