CVE-2020-15888: Out-of-bounds Read (CWE-125)
value: Lua through 5.4.0 mishandles the interaction between stack resizes and garbage collection, leading to a heap-based buffer overflow, heap-based buffer over-read, or use-after-free.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 8.8
Published: 2020-07-21T22:15Z
Repository: https://github.com/lua/lua
Language: C
Files changed: ldo.c, ldo.h, ltm.c, lvm.c
Commit date: 2020-07-07 18:03:48 -0300
Commit message: Fixed bugs of stack reallocation x GC

Macro 'checkstackGC' was doing a GC step after resizing the stack;
the GC could shrink the stack and undo the resize. Moreover, macro
'checkstackp' also does a GC step, which could remove the preallocated
CallInfo when calling a function. (Its name has been changed to
'checkstackGCp' to emphasize that it calls the GC.)

Diff: diff --git a/ldo.c b/ldo.c
index 66217a4b..e3db1f74 100644
--- a/ldo.c
+++ b/ldo.c
@@ -463,17 +463,17 @@ void luaD_call (lua_State *L, StkId func, int nresults) {
       goto Cfunc;
     case LUA_VLCF:  /* light C function */
       f = fvalue(s2v(func));
      Cfunc: {
       int n;  /* number of returns */
-      CallInfo *ci = next_ci(L);
-      checkstackp(L, LUA_MINSTACK, func);  /* ensure minimum stack size */
+      CallInfo *ci;
+      checkstackGCp(L, LUA_MINSTACK, func);  /* ensure minimum stack size */
+      L->ci = ci = next_ci(L);
       ci->nresults = nresults;
       ci->callstatus = CIST_C;
       ci->top = L->top + LUA_MINSTACK;
       ci->func = func;
-      L->ci = ci;
       lua_assert(ci->top <= L->stack_last);
       if (L->hookmask & LUA_MASKCALL) {
         int narg = cast_int(L->top - func) - 1;
         luaD_hook(L, LUA_HOOKCALL, -1, 1, narg);
       }
@@ -483,16 +483,17 @@ void luaD_call (lua_State *L, StkId func, int nresults) {
       api_checknelems(L, n);
       luaD_poscall(L, ci, n);
       break;
     }
     case LUA_VLCL: {  /* Lua function */
-      CallInfo *ci = next_ci(L);
+      CallInfo *ci;
       Proto *p = clLvalue(s2v(func))->p;
       int narg = cast_int(L->top - func) - 1;  /* number of real arguments */
       int nfixparams = p->numparams;
       int fsize = p->maxstacksize;  /* frame size */
-      checkstackp(L, fsize, func);
+      checkstackGCp(L, fsize, func);
+      L->ci = ci = next_ci(L);
       ci->nresults = nresults;
       ci->u.l.savedpc = p->code;  /* starting point */
       ci->callstatus = 0;
       ci->top = func + 1 + fsize;
       ci->func = func;
@@ -502,11 +503,11 @@ void luaD_call (lua_State *L, StkId func, int nresults) {
       lua_assert(ci->top <= L->stack_last);
       luaV_execute(L, ci);  /* run the function */
       break;
     }
     default: {  /* not a function */
-      checkstackp(L, 1, func);  /* space for metamethod */
+      checkstackGCp(L, 1, func);  /* space for metamethod */
       luaD_tryfuncTM(L, func);  /* try to get '__call' metamethod */
       goto retry;  /* try again with metamethod */
     }
   }
 }
diff --git a/ldo.h b/ldo.h
index 7760f853..6c6cb285 100644
--- a/ldo.h
+++ b/ldo.h
@@ -15,10 +15,12 @@
 
 /*
 ** Macro to check stack size and grow stack if needed.  Parameters
 ** 'pre'/'pos' allow the macro to preserve a pointer into the
 ** stack across reallocations, doing the work only when needed.
+** It also allows the running of one GC step when the stack is
+** reallocated.
 ** 'condmovestack' is used in heavy tests to force a stack reallocation
 ** at every check.
 */
 #define luaD_checkstackaux(L,n,pre,pos)  \
 	if (L->stack_last - L->top <= (n)) \
@@ -33,20 +35,20 @@
 #define savestack(L,p)		((char *)(p) - (char *)L->stack)
 #define restorestack(L,n)	((StkId)((char *)L->stack + (n)))
 
 
 /* macro to check stack size, preserving 'p' */
-#define checkstackp(L,n,p)  \
+#define checkstackGCp(L,n,p)  \
   luaD_checkstackaux(L, n, \
     ptrdiff_t t__ = savestack(L, p);  /* save 'p' */ \
     luaC_checkGC(L),  /* stack grow uses memory */ \
     p = restorestack(L, t__))  /* 'pos' part: restore 'p' */
 
 
 /* macro to check stack size and GC */
 #define checkstackGC(L,fsize)  \
-	luaD_checkstackaux(L, (fsize), (void)0, luaC_checkGC(L))
+	luaD_checkstackaux(L, (fsize), luaC_checkGC(L), (void)0)
 
 
 /* type of protected functions, to be ran by 'runprotected' */
 typedef void (*Pfunc) (lua_State *L, void *ud);
 
diff --git a/ltm.c b/ltm.c
index ae60983f..4770f96b 100644
--- a/ltm.c
+++ b/ltm.c
@@ -238,11 +238,11 @@ void luaT_adjustvarargs (lua_State *L, int nfixparams, CallInfo *ci,
                          const Proto *p) {
   int i;
   int actual = cast_int(L->top - ci->func) - 1;  /* number of arguments */
   int nextra = actual - nfixparams;  /* number of extra arguments */
   ci->u.l.nextraargs = nextra;
-  checkstackGC(L, p->maxstacksize + 1);
+  luaD_checkstack(L, p->maxstacksize + 1);
   /* copy function to the top of the stack */
   setobjs2s(L, L->top++, ci->func);
   /* move fixed parameters to the top of the stack */
   for (i = 1; i <= nfixparams; i++) {
     setobjs2s(L, L->top++, ci->func + i);
@@ -257,11 +257,11 @@ void luaT_adjustvarargs (lua_State *L, int nfixparams, CallInfo *ci,
 void luaT_getvarargs (lua_State *L, CallInfo *ci, StkId where, int wanted) {
   int i;
   int nextra = ci->u.l.nextraargs;
   if (wanted < 0) {
     wanted = nextra;  /* get all extra arguments available */
-    checkstackp(L, nextra, where);  /* ensure stack space */
+    checkstackGCp(L, nextra, where);  /* ensure stack space */
     L->top = where + nextra;  /* next instruction will need top */
   }
   for (i = 0; i < wanted && i < nextra; i++)
     setobjs2s(L, where + i, ci->func - nextra + i);
   for (; i < wanted; i++)   /* complete required results with nil */
diff --git a/lvm.c b/lvm.c
index ccbfbab5..d78d6be2 100644
--- a/lvm.c
+++ b/lvm.c
@@ -1632,11 +1632,11 @@ void luaV_execute (lua_State *L, CallInfo *ci) {
           lua_assert(base == ci->func + 1);
         }
         while (!ttisfunction(s2v(ra))) {  /* not a function? */
           luaD_tryfuncTM(L, ra);  /* try '__call' metamethod */
           b++;  /* there is now one extra argument */
-          checkstackp(L, 1, ra);
+          checkstackGCp(L, 1, ra);
         }
         if (!ttisLclosure(s2v(ra))) {  /* C function? */
           luaD_call(L, ra, LUA_MULTRET);  /* call it */
           updatetrap(ci);
           updatestack(ci);  /* stack may have been relocated */

Vulnerable code:       CallInfo *ci = next_ci(L);
      checkstackp(L, LUA_MINSTACK, func);  /* ensure minimum stack size */
      L->ci = ci;
      CallInfo *ci = next_ci(L);
      checkstackp(L, fsize, func);
      checkstackp(L, 1, func);  /* space for metamethod */
#define checkstackp(L,n,p)  \
	luaD_checkstackaux(L, (fsize), (void)0, luaC_checkGC(L))
  checkstackGC(L, p->maxstacksize + 1);
    checkstackp(L, nextra, where);  /* ensure stack space */
          checkstackp(L, 1, ra);
Fixed code:       CallInfo *ci;
      checkstackGCp(L, LUA_MINSTACK, func);  /* ensure minimum stack size */
      L->ci = ci = next_ci(L);
      CallInfo *ci;
      checkstackGCp(L, fsize, func);
      L->ci = ci = next_ci(L);
      checkstackGCp(L, 1, func);  /* space for metamethod */
** It also allows the running of one GC step when the stack is
** reallocated.
#define checkstackGCp(L,n,p)  \
	luaD_checkstackaux(L, (fsize), luaC_checkGC(L), (void)0)
  luaD_checkstack(L, p->maxstacksize + 1);
    checkstackGCp(L, nextra, where);  /* ensure stack space */
          checkstackGCp(L, 1, ra);