Small inconsistency?

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Small inconsistency?

Thomas Lauer-3
Here's what appears to be an inconsistency in the Lua sources.

This comes from lua.h:
	/* mark for precompiled code (`<esc>Lua') */
	#define	LUA_SIGNATURE	"\033Lua"

In lundump.h, there's:
	/* size of header of binary files */
	#define LUAC_HEADERSIZE		 12

And finally, from lundump.c:
	static void LoadHeader(LoadState* S)
	{
	 char h[LUAC_HEADERSIZE];
	 char s[LUAC_HEADERSIZE];
	 luaU_header(h);
	 LoadBlock(S,s,LUAC_HEADERSIZE);
	 IF (memcmp(h,s,LUAC_HEADERSIZE)!=0, "bad header");
	}

... as well as:
	void luaU_header (char* h)
	{
	 int x=1;
	 memcpy(h,LUA_SIGNATURE,sizeof(LUA_SIGNATURE)-1);
	 h+=sizeof(LUA_SIGNATURE)-1;
	 *h++=(char)LUAC_VERSION;
	 *h++=(char)LUAC_FORMAT;
	 *h++=(char)*(char*)&x;		 	 	 /* endianness */
	 *h++=(char)sizeof(int);
	 *h++=(char)sizeof(size_t);
	 *h++=(char)sizeof(Instruction);
	 *h++=(char)sizeof(lua_Number);
	 *h++=(char)(((lua_Number)0.5)==0);		 /* is lua_Number integral? */
	}

What happens if someone changes LUA_SIGNATURE in lua.h to something longer than
4 characters? Well, I did (after checking all obvious places where the
signature's used) and recompiled. To my surprise, the resulting luac executable
crashed... so I started looking at some less obvious places and found that these
functions in lundump.c simply suppose a signature length of 4 characters or less.

Perhaps there should be a warning in lua.h? Or somewhat more robust code in
lundump.c?


Reply | Threaded
Open this post in threaded view
|

Re: Small inconsistency?

Daniel Silverstone
On Mon, 2007-03-19 at 16:25 +0000, Thomas Lauer wrote:
> Perhaps there should be a warning in lua.h? Or somewhat more robust code in
> lundump.c?
> 

Perhaps instead of:
	#define LUAC_HEADERSIZE		 12

We should have:

#define LUAC_HEADERSIZE (sizeof(LUA_SIGNATURE) + 8)

-- 
Daniel Silverstone                         http://www.digital-scurf.org/
PGP mail accepted and encouraged.            Key Id: 2BC8 4016 2068 7895



Reply | Threaded
Open this post in threaded view
|

Re: Small inconsistency?

Luiz Henrique de Figueiredo
In reply to this post by Thomas Lauer-3
> Here's what appears to be an inconsistency in the Lua sources.

Not really an inconsistency, I think. Perhaps the code is not as defensive
as you wish. If you change LUA_SIGNATURE or the things that go into the
header, you must set LUAC_HEADERSIZE accordingly.

A simple (untested) fix, which won't catch all abuse is to modify luaU_header
as follows:

void luaU_header (char* h)
{
 int x=1;
 char *H=h;					<<<< new
 ...
 lua_assert((h-H)==LUAC_HEADERSIZE);		<<<< new
}

--lhf

Reply | Threaded
Open this post in threaded view
|

Re: Small inconsistency?

Luiz Henrique de Figueiredo
In reply to this post by Daniel Silverstone
> #define LUAC_HEADERSIZE (sizeof(LUA_SIGNATURE) + 8)

It should be (sizeof(LUA_SIGNATURE)-1+8) because the trailing NUL in
LUA_SIGNATURE is not part of the header. However, this does not catch
any change you may make to the header.
--lhf

Reply | Threaded
Open this post in threaded view
|

Re: Small inconsistency?

Thomas Lauer-3
In reply to this post by Thomas Lauer-3
Luiz Henrique de Figueiredo wrote:

> Not really an inconsistency, I think. Perhaps the code is not as defensive
> as you wish.

Code, including my own, is never as defensive as I wish:-)

> If you change LUA_SIGNATURE or the things that go into the
> header, you must set LUAC_HEADERSIZE accordingly.

Well, if I *must* set LUAC_HEADERSIZE accordingly then a hint in lua.h would be
appropriate, methinks. And that was all my post was about.