[fzipi]

what

Fixes the return values of l_setvar() (apache2/msc_lua.c), the v2 implementation of Lua m.setvar():

• The two error paths (wrong number of arguments, no collection in the name) return 0 instead of -1.
• The success path returns 0 instead of the status of msre_action_setvar_execute().
• The NULL check after luaL_checkstring() is removed: it can't trigger, because luaL_checkstring() raises a Lua error instead of returning NULL.
• The typo in the debug message is fixed: "funtion must has" is now "function must have".

why

A lua_CFunction returns the number of values it pushed, so Lua handed the script whatever was on the stack.

• On success, msre_action_setvar_execute() returns 1. On unpatched v2/master, local r = m.setvar("tx.a", "x") therefore gives r == "x".
• On the error paths, -1 is undefined behaviour in Lua's call machinery. v3: Lua m.setvar() returns -1 on errors, so the script gets stack garbage #3731 shows the v3 equivalent handing the script the transaction pointer.

The lua_pop(L, 2) stays. Unlike in v3, it pops the __msr and __rule globals that l_setvar() pushed, not the arguments, so var_value stays valid.

tests

There are three new cases in tests/regression/rule/00-script.t, with the scripts in server_root/conf/ (force-added, like test.lua and match.lua, because that directory is in .gitignore):

• no collection, which must log the error and return nil;
• one argument, which must log the error and return nil;
• a successful TX set, which must return nil and set tx.a.

Tested in Docker (Ubuntu 24.04, Apache 2.4.58, Lua 5.1.5): rule/00-script.t gives 6 passed and 0 failed. With the msc_lua.c change reverted, all three new cases fail.

references

• v2 counterpart of v3: Lua m.setvar() returns -1 on errors, so the script gets stack garbage #3731, which is fixed for v3 in fix: Lua m.setvar() returned -1, handing the script stack garbage #3732

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5cfaac02-4dc9-4323-90dc-bb17bd4868b5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
2 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

🟢 Approval recommended

No active review issues were identified for the Lua m.setvar() return-handling fix.

No open issues 🎉

What changed in this PR

Fixes ModSecurity v2’s Lua m.setvar() return handling, addressing the v2 counterpart of #3731 without changing variable-setting behavior.

Changes:

• Returns zero Lua results on success and validation failures, preventing stack values from reaching scripts.
• Removes an unreachable null check and corrects the argument-count diagnostic.
• Adds regression coverage for missing collections, incorrect argument counts, and successful TX variable updates.

File	Description
tests/​regression/​server_root/​conf/​setvar-tx.lua	Logs the return value and updated TX variable for verification.
tests/​regression/​server_root/​conf/​setvar-error-onearg.lua	Exercises a call with a missing argument.
tests/​regression/​server_root/​conf/​setvar-error-nodot.lua	Exercises a variable name without a collection.
tests/​regression/​rule/​00-script.t	Adds assertions for all three Lua regression cases.
apache2/​msc_lua.c	Corrects return counts, removes the unreachable check, and fixes diagnostic wording.

_{Review effort: Balanced}