Skip to content

Commit

Permalink
Disable JIT optimization for user code and allow string.find in plain…
Browse files Browse the repository at this point in the history 
… mode

Disabling LuaJIT for user code enables normal working of debug.sethook() even for loops. The drawback is that that code will run more slowly.

The fourth parameter of string.find indicates whether the second parameter should be interpreted literally (true) or as a pattern (false). Allowing patterns enables DoS attacks, but it's possible to
allow literal matching with little effort, by disallowing the function only if the fourth parameter (plain mode) is not `true`.
  • Loading branch information
@Jeija
Pedro Gimeno authored and Jeija committed Apr 2, 2016
1 parent 72e513e commit b487783
Showing 1 changed file with 21 additions and 35 deletions.
56 changes: 21 additions & 35 deletions mesecons_luacontroller/init.lua
View file
Expand Up @@ -216,6 +216,17 @@ local function safe_string_rep(str, n)
return string.rep(str, n)
end

-- string.find with a pattern can be used to DoS the server.
-- Therefore, limit string.find to patternless matching.
local function safe_string_find(...)
if (select(4, ...)) ~= true then
debug.sethook() -- Clear hook
error("string.find: 'plain' (fourth parameter) must always be true in a LuaController")
end

return string.find(...)
end

local function remove_functions(x)
local tp = type(x)
if tp == "table" then
Expand Down Expand Up @@ -292,6 +303,7 @@ local function create_environment(pos, mem, event)
rep = safe_string_rep,
reverse = string.reverse,
sub = string.sub,
find = safe_string_find,
},
math = {
abs = math.abs,
Expand Down Expand Up @@ -354,22 +366,6 @@ local function timeout()
end


local function code_prohibited(code)
-- LuaJIT doesn't increment the instruction counter when running
-- loops, so we have to sanitize inputs if we're using LuaJIT.
if not rawget(_G, "jit") then
return false
end
local prohibited = {"while", "for", "repeat", "until", "goto"}
code = " "..code.." "
for _, p in ipairs(prohibited) do
if string.find(code, "[^%w_]"..p.."[^%w_]") then
return "Prohibited command: "..p
end
end
end


local function create_sandbox(code, env)
if code:byte(1) == 27 then
return nil, "Binary code prohibited."
Expand All @@ -378,24 +374,17 @@ local function create_sandbox(code, env)
if not f then return nil, msg end
setfenv(f, env)

-- Turn off JIT optimization for user code so that count
-- events are generated when adding debug hooks
if rawget(_G, "jit") then
jit.off(f, true)
end

return function(...)
-- Normal Lua: Use instruction counter to stop execution
-- after luacontroller_maxevents.
-- LuaJIT: Count function calls instead of instructions, allows usage
-- of function keyword. However, LuaJIT still doesn't trigger
-- lines events when using infinite loops.
-- Use instruction counter to stop execution
-- after luacontroller_maxevents
local maxevents = mesecon.setting("luacontroller_maxevents", 10000)
if not rawget(_G, "jit") then
debug.sethook(timeout, "", maxevents)
else
local events = 0
debug.sethook(function ()
events = events + 1
if events > maxevents then
timeout()
end
end, "c")
end
debug.sethook(timeout, "", maxevents)
local ok, ret = pcall(f, ...)
debug.sethook() -- Clear hook
if not ok then error(ret, 0) end
Expand Down Expand Up @@ -432,9 +421,6 @@ local function run(pos, event)
local mem = load_memory(meta)
local code = meta:get_string("code")

local err = code_prohibited(code)
if err then return err end

-- Create environment
local env = create_environment(pos, mem, event)

Expand Down

1 comment on commit b487783

@Jeija
Copy link
Collaborator

@Jeija Jeija commented on b487783 Apr 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, okay, that is a valid criticism; I didn't think of that. I'd still like to keep the message as it is, as the number of parameters is often counted including the object reference as the first parameter in many debug / error messages (so people are used to this way of counting), and second because plain could also be interpreted as just a word if not knowing the parameters of string.find by their name.

Please sign in to comment.