lua-users home
lua-l archive

[Date Prev][Date Next][Thread Prev][Thread Next] [Date Index] [Thread Index]


It was thus said that the Great Sir Pogsalot once stated:
> Er, I don't agree with either of these arguments...
> 
> Coda:  I'm creating potentially 1000 userdata objects with associated
> private userdata for a *small* server.  An extra table on each would be
> noticeable..
> 
> Sean: So... I should get the other person to change their code to avoid the
> rawlen() to allow for evil users to segfault from within their library?

  Yes.  

  We're not talking about feeding code something malicious from outside
(like through specially crafted network packet or input file) but code meant
to be used by another programmer (who you are calling "user" but I'm reading
as "programmer").

>  Personally I don't understand how you would not typecheck with a
> luaL_checkudata() and not do the responsible rawlen() size check...  That's
> not defensive programming that's just reasonable. :\

  In my opinion, that's defensive programming.  So let's walk through your
case.

	foo__t *p = luaL_checkudata(L,1,TYPE_FOO);
	if (lua_rawlen(L,1) != sizeof(foo__t))
	{
	  lua_pushboolean(L,0);
	  return 1;
	}

  Yes, you may have prevented *THAT* particular use.  Fine.  As a malicious
user though, I can do this:

	void *p = lua_newuserdata(L,sizeof(foo__t));
	memset(p,0xCC,sizeof(foo__t)); /* [2] */
	luaL_getmetatable(L,TYPE_FOO);
	lua_setmetatable(L,-2);
	/* muahahahahahahaha! */

  Yeah, that lua_rawlen() *really* saved you in the face of a real malicious
programming, didn't it?  Okay, double down on things ...

	foo__t *p = luaL_checkudata(L,1,TYPE_FOO);
	if (
                (lua_rawlen(L,1) != sizeof(foo__t))
	     || (p->magicvalue   != FOOMAGIC)
	)
	{
	  lua_pushboolean(L,0);
	  return 1;
	}

  Curses!  Foiled again!  No matter!

	foo__t *p = lua_newuserdata(L,sizeof(foo__t));
	memset(p,0xCC,sizeof(foo__t));
	p->magicvalue = FOOMAGIC;
	luaL_getmetatable(L,TYPE_FOO);
	lua_setmetatable(L,-2);
	/* Muahahahahahahahahaha! */

  You aren't stopping "evil users" as much as "adding pointless security
theater code."

  Okay, but what about the stupid programmers?  Sorry, you just end up
hiding bugs.  Notice how I return false there?  A program might run for ages
before that "false" becomes a problem.  Where did it come from?  Why is this
popping up?  How long will I waste looking for the root cause?

  Sure, I could have done a lua_pushnil(L) and maybe the issue comes up
earlier.  Maybe.  Or it's plastered over with nil checks in the Lua code
(because, you know, being reasonable here).  Or is it that you do not trust
yourself with your own code?

  No, I believe in "trust the programmer."  In the code, I'm asking for a
userdata of type TYPE_FOO.  It's your own damn fault if you create a
userdata with something other than a foo__t and tell Lua it's a TYPE_FOO. 
Not mine.  My code will crash, hard.  And you will be forced to find the
bug.  My code laid out the contract; it's up to you to follow it.

  It's explained better in _Writing Solid Code_ [1].

  Oh, one more thing---we manged to get along quite well without
lua_rawlen() until Lua 5.2.  

  Yup, might as well burn Lua 5.1 because you can't double check the
userdata length with lua_rawlen().  Man, what an oversight; damn, it's good
no one is allowed to use Lua 5.1 anymore [4].

  Okay, seriously, as long as that library gets a pointer to a foo__t, then
it's all good.  Lua is managing the memory, so the fact that the library is
getting a larger foo__t isn't an issue.  And you get your private fields. 
And you get segfaults faster so it's easier to debug.

  -spc (Win win all around!)

[1]	_Writing Solid Code_ by Steve Maguire goes into this in depth. It
	was one of only two books I've read over the past 25 years that
	fundamentally changed how I write code.

[2]	Why 0xCC and not 0?  Because segfaults are too easy.  For non-Intel
	hardware, a better choice might be 0xBB, although for Motorola 32bit
	CPUs 0xAD or 0xFD make good choices.  [3]

[3]	Bonus points as to why these particular values.

[4]	I do hope the sarcasm comes through.