Re: Potential Null Pointer Dereference in resizebox method from lauxlib.c

[Yongchao Wang <chaowyc@...>]

Hi,

I get your point.

After initiating the thread, I conducted a thorough examination of all the call sites for the two wrapper functions of `touserdata`, namely `lua_touserdata` and `lua_topointer`, across the project. As a result, I detected three more null pointer dereferences that are similar in nature (I will present them later).

Furthermore, I observed that some of the call sites handled the return value of these functions appropriately, while others did not, including the `resizebox` function, regardless of whether the call site was static or not.

Considering these inconsistencies, it is likely that the software will become harder to maintain and could potentially lead to crashes. Therefore, I suggest addressing these two potential null pointer dereferences by properly checking the return value of the functions, as has been done at other call sites. (An example fix for the similar issue is provided.)

Three more NPD were caused by calling `lua_touserdata`

First:

Call  `lua_touserdata` at lmathlib.c:560. The return value is directly dereferenced at lmathlib.c:561

static int math_random (lua_State *L) {
   lua_Integer low, up;
   lua_Unsigned p;
   RanState *state = (RanState *)lua_touserdata(L, lua_upvalueindex(1)); line 560 
   Rand64 rv = nextrand(state->s); /* next pseudo-random value */ line 561
   ...
}

Second:
Call  `lua_touserdata` at lmathlib.c:619. The return value is directly dereferenced in lmathlib.c:621
static int math_randomseed (lua_State *L) {
   RanState *state = (RanState *)lua_touserdata(L, lua_upvalueindex(1)); //619
   if (lua_isnone(L, 1)) {
      randseed(L, state); // 621
   }
   ...
}

Third:
Call  `lua_touserdata` at lstrlib.c:839. The return value is directly dereferenced at 841

static int gmatch_aux (lua_State *L) {
   GMatchState *gm = (GMatchState *)lua_touserdata(L, lua_upvalueindex(3)); // line 839
   const char *src;
   gm->ms.L = L; // line 841
   for (src = "" src <= gm->ms.src_end; src++) {
   const char *e;
   reprepstate(&gm->ms);
   ...
}

I also list an example fix in a static method for the wrapper `lua_topointer` for your reference:

1. Return null from lapi.c:480 and lapi.c:485

LUA_API const void *lua_topointer (lua_State *L, int idx) {
   const TValue *o = index2value(L, idx);
   switch (ttypetag(o)) {
      case LUA_VLCF: return cast_voidp(cast_sizet(fvalue(o)));
      case LUA_VUSERDATA: case LUA_VLIGHTUSERDATA:
         return touserdata(o); // line 480 return null
      default: {
         if (iscollectable(o))
            return gcvalue(o);
         else
            return NULL; // line 485 return null
       }
    }
}

2. `lua_topointer` is called at lstrlib.c:1334 and the return value is checked at lstrlib.c:1336

static int str_format (lua_State *L) {
...
case 'p': {
   const void *p = lua_topointer(L, arg); // line 1334
   checkformat(L, form, L_FMTFLAGSC, 0);
   if (p == NULL) { /* avoid calling 'printf' with argument NULL */ // line 1336
      p = "(null)"; /* result */
      form[strlen(form) - 1] = 's'; /* format it as a string */
   }
   nb = l_sprintf(buff, maxitem, form, p);
   break;
  }
...
}

Best regards

Yongchao

On Sat, 22 Jul 2023 at 11:25, 云风 <cloudwu@gmail.com> wrote:

But resizebox is a static function, it’s used only by luaL_Buffer, and user cannot call it directly.

发自我的 iPhone

> 在 2023年7月22日，10:08，Yongchao Wang <chaowyc@gmail.com> 写道：
>
> 
> Hi all,
> We have detected that the resizebox method may trigger a null pointer dereference. Here is a possible vulnerable trace:
>
> 1. Return null to caller at lapi.c:457
> // From lapi.c
> l_sinline void *touserdata (const TValue *o) {
>   switch (ttype(o)) {
>     case LUA_TUSERDATA: return getudatamem(uvalue(o));
>     case LUA_TLIGHTUSERDATA: return pvalue(o);
>     default: return NULL;  // line 457 Return null
>   }
> }
> 2. Return the return value of function touserdata, could be null, to caller at lapi.c:462.
> //From lapi.c
> LUA_API void *lua_touserdata (lua_State *L, int idx) {
>   const TValue *o = index2value(L, idx);
>   return touserdata(o); // line 462 Return null
> }
>
> 3. Function lua_touserdata executes and stores the return value to box (box can be null) at lauxlib.c:476
> and load value from box->size at lauxlib.c:477, which will lead to null pointer dereference
> // From lauxlib.c
> static void *resizebox (lua_State *L, int idx, size_t newsize) {
>   void *ud;
>   lua_Alloc allocf = lua_getallocf(L, &ud);
>   UBox *box = (UBox *)lua_touserdata(L, idx);  // line 476 box could be null
>   void *temp = allocf(ud, box->box, box->bsize, newsize); // line 477 dereference box
>   if (l_unlikely(temp == NULL && newsize > 0)) {  /* allocation error? */
>     lua_pushliteral(L, "not enough memory");
>     lua_error(L);  /* raise a memory error */
>   }
>   box->box = temp;
>   box->bsize = newsize;
>   return temp;
> }
>
> This could lead to a program crash or other unwanted behavior. Please fix it as soon as possible.
> Best
> Yongchao