v3: Lua m.setvar() returns -1 on errors, so the script gets stack garbage

Open Beginner friendly
#3,731 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

@fzipi is already working on this.

Since Nov 1, 2026.

  • #3732 by @fzipi — open

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
72/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp, lua
Domain
backend

Research direction

Start in src/engine/lua.cc at Lua::setvar(), where the three error paths return -1 (around lines 343, 351 and 366). Compare with getvar(), which calls luaL_checkstring() before any C++ locals exist. Done means the error paths return 0 (or push nil and return 1), the two luaL_checkstring() calls move ahead of the std::string locals, the typo is fixed, and a regression test runs the two reproduction scripts and checks the returned value.

Written by the indexing model from the issue text.

Description

3.x bug

@airween, I found this while reviewing #3730. It's in the same file but a separate problem, so I'm filing it on its own.

Problem

Lua::setvar() (src/engine/lua.cc, v3/master 9d45c6d7) returns -1 on its three error paths:

  • line 343: wrong number of arguments;
  • line 351: luaL_checkstring() returned NULL;
  • line 366: no . in the name, so no collection.

A lua_CFunction must return the number of values it pushed onto the stack. Lua doesn't check for a negative count in release builds, so the call returns whatever happens to be on the stack.

Reproduction

I used two scripts run with exec: in phase 1 (macOS, Lua 5.5.1):

-- nodot.lua
function main()
    local r = m.setvar("novar", "x");
    m.log(4, "after setvar, got: " .. tostring(r));
    return nil;
end
-- onearg.lua
function main()
    local r = m.setvar("tx.a");
    m.log(4, "after setvar, got: " .. tostring(r));
    return nil;
end

Debug log:

[8] m.setvar: Must specify a collection using dot character - ie m.setvar(tx.myvar,mydata)
[4] after setvar, got: userdata: 0x16f5b2498
...
[8] m.setvar: Failed m.setvar funtion must has 2 arguments
[4] after setvar, got: [

In the first case the script gets back the __transaction pointer that setvar() pushed. In the second it gets a stray string. Nothing crashed here, but this is undefined behaviour, and the result depends on the Lua version and build.

A related, smaller point

luaL_checkstring() raises a Lua error, using longjmp with the usual C build of Lua, when an argument isn't a string, e.g. m.setvar("tx.a", {}). In setvar() that happens after the three std::string locals have been constructed, so their destructors are skipped. They're still empty at that point, so nothing leaks today, but it's fragile. getvar(), getvars() and log() call luaL_checkstring() before any C++ object exists, so they're fine.

Suggested fix
  • Return 0 (no results) on the three error paths, as log() does. Or, if a script should see the failure, push nil or false and return 1.
  • Move the two luaL_checkstring() calls before the std::string locals, as in getvar().
  • While there: fix the typo in the debug message ("funtion must has" → "function must have").

A regression case with the two scripts above, checking that m.setvar() returns nil (or false), would cover it.

Dominant language
C++
Stars
9.8k
Forks
1.8k
Avg merge
1d 9h
Merged PRs (30d)
19

Getting set up

  • No Dockerfile or Docker Compose file
  • Has a pull request template
  • No contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from owasp-modsecurity/ModSecurity

All issues in owasp-modsecurity/ModSecurity

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.