Project

General

Profile

Bug #2938

Lighttpd crashes on wrong return type in lua script

Added by flynn 6 months ago. Updated 5 months ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
Category:
mod_magnet
Target version:
Start date:
2019-03-06
Due date:
% Done:

100%

Estimated time:
Missing in 1.5.x:

Description

If a lua script is attached with magnet.attract-raw-url-to, this script is expected to either return nothing or a numeric value, which is interpreted as http status code later.

If a boolean is returned, lighttpd crashes with a segfault. This is a lua programming error, but lighttpd should not crash.

Related log message in error log is:
(mod_magnet.c.420) (lua-atpanic) bad argument #-1 (number expected, got boolean)

gdb backtrace ist very short:
Program received signal SIGSEGV, Segmentation fault.
_longjmp_chk () at ../sysdeps/unix/sysv/linux/x86_64/_longjmp_chk.S:167
167 ../sysdeps/unix/sysv/linux/x86_64/____longjmp_chk.S: Datei oder Verzeichnis nicht gefunden.
(gdb) bt
#0 _longjmp_chk () at ../sysdeps/unix/sysv/linux/x86_64/_longjmp_chk.S:167
#1 0x65479cc994dcfd30 in ?? ()
Backtrace stopped: Cannot access memory at address 0x65479cc994dcfd30

Steps to reproduce:
- lighttpd.conf with mod_magnet and a lua script attached with magnet.attract-raw-url-to
- lua script with only one line:

  return true

- request to the url with attached lua script

Associated revisions

Revision 8064b748 (diff)
Added by gstrauss 6 months ago

[mod_magnet] fix invalid script return-type crash (fixes #2938)

(thx flynn)

x-ref:
"Lighttpd crashes on wrong return type in lua script"
https://redmine.lighttpd.net/issues/2938

History

#1

Updated by gstrauss 6 months ago

  • Target version changed from 1.4.54 to 1.4.x
#2

Updated by gstrauss 6 months ago

Based on your description, please have a look at mod_magnet.c line 968:

        lua_return_value = (int) luaL_optinteger(L, -1, -1);

That code expects an int and is possibly what is resulting in the lua panic, but I don't have time at this moment to test that theory. The code should probably verify the existence and type of argument at that position on the stack before calling luaL_optinteger().

#3

Updated by flynn 6 months ago

This patch fixes the problem for me:

- behaviour with numeric and nil types is unchanged
- all other types are handled like nil type (return -1), but a log messages is written and the crash is avoided

#4

Updated by gstrauss 6 months ago

Should lua_return_type == LUA_TNIL be an error, too? This is a programming interface. If someone is not returning a value per the API, then we should probably not assume that everything is ok without at least logging.

#5

Updated by flynn 6 months ago

From my current understanding and usage of lua scripts with mod_magnet there two use cases:

1.) generating content with lua, redirect the request or request authentication
In these cases the lua script must return a valid http status code and maybe additional header and/or content

Returning a number means: the request ends here

2.) the lua script just modifies/verifies the request and has nothing to change.
Take a look at the traffic quota example in the AbsoLUAtion page of this wiki.
If the request is not blocked, the script just ends returning nothing, which is nil in Lua.
It does not make a difference, whether nil is returned explicit or the script just ends (I just verified both cases).
Another use case: the lua script checks for authentication:
- either the request is missing the the needed data (e.e. header), than we have case 1.), a redirect to a login url or a status 401
- or the request is verified and can proceed

Returning nothing/nil in a lua script just means: continue with the request

If returning nil is considred an error, this would break a lot of existing scripts.

The example "Do basic HTTP-Auth against a MySQL DB/Table" in the AbsoLUAtion page ends with the follwing two lines

-- return nothing to proceed normal operation
return
#6

Updated by gstrauss 6 months ago

#7

Updated by gstrauss 6 months ago

  • Status changed from New to Patch Pending
  • Target version changed from 1.4.x to 1.4.54
#8

Updated by gstrauss 5 months ago

  • Status changed from Patch Pending to Fixed
  • % Done changed from 0 to 100

Also available in: Atom