Re: A proposal for safe bytecode in Lua 5.2

[Peter Cawley <lua@...>]

On Fri, Apr 8, 2011 at 3:00 PM, Roberto Ierusalimschy
<roberto@inf.puc-rio.br> wrote:
>> I'm now looking for feedback on the proposal: Do you think it is a
>> good proposal? Do the Lua authors think that inclusion of such a
>> verifier is possible? Should I go ahead with spending time trying to
>> break it? If the general consensus is positive, then I shall go ahead,
>> and subsequently post a patch once I'm happy with it.
>
> It sounds very good, but I do not think it will go into 5.2. It does
> not looks mature enough (and we would have to understand it deeply to
> go into the distribution ;).
>
> Nevertheless, it seems interesting enough to deserve some support. We
> could do the small changes and add appropriate macros so that users
> could compile Lua with your verifier without having to change any
> source.
>
> Some questions about those changes:
>
>> * Change the bytecode format to make startpc and endpc values for
>> every local compulsory information rather than optional debug
>> information (the name of the local remains as optional debug
>> information).
>
> Couldn't you simply reject source that do not have that information?

Indeed you could; my phrasing should have been "compulsory if you want
verification".

>> * Reintroduce some checks which were present in 5.1: the
>> non-negativity check in lundump.c's LoadInt, and the table type check
>> in lvm.c's OP_SETLIST (as efficiently verifying the latter seems
>> non-trivial).
>
> I undestand the OP_SETLIST test, but shouldn't be easy for the verifier
> to test whether integers are non-negative?

It certainly could, but LoadInt is also frequently called during the
undumping process and the result passed on to luaM_newvector. In these
cases, if LoadInt returned -1, then potentially bad things happen.
Hence this check is required for correct undumping, and rather than
perform it in the undumper between each LoadInt/luaM_newvector pair,
it seems more sensible to perform it in every call to LoadInt. Doing
so also saves the verifier from having to perform some non-negativity
checks.

>> * Two minor tweaks to the code generator: [...] and when the "local
>> function" sugar is being compiled, shift the startpc value of the new
>> local to not include the OP_CLOSURE (which makes it consistent with
>> the non-sugared behaviour).
>
> I did not understand this part. The non-sugared behaviour would be this:
>
>  local f
>  f = function (...) ... end
>
> How the new local 'f' cannot include the OP_CLOSURE that follows its
> declaration?

The change I made was to add "getlocvar(fs, fs->nactvar - 1)->startpc
= fs->pc;" to the end of "localfunc". Looking back at the Lua 5.1.4
source, a very similar line appears in its "localfunc". Without this
line, the local variable holding the resulting closure becomes active
whilst holding garbage. This presents a problem for the verifier, as
to prevent information leakage, it rejects the code if a local
variable could ever hold garbage. As alluded to in the 5.1 source, it
also presents a problem for the debug API; in the following code, the
debug API sees a non-temporary local variable outside of "secret"
holding "super secret value".

-------- begin code --------
function public()
  secret()
  local dummy
  local function local_func() end
end

function secret()
  local x = "super secret value"
end

function hook()
  if debug.getinfo(2, "f").func == public then
    print("--- begin public locals ---")
    local n = 1
    while true do
      local name, val = debug.getlocal(2, n)
      if not name then
        break
      end
      print(n, name, val)
      n = n + 1
    end
    print("---  end  public locals ---")
  end
end

debug.sethook(hook, "", 1)
public()
--------  end  code --------