CVE-2020-15945: Other (NVD-CWE-Other)
value: Lua through 5.4.0 has a segmentation fault in changedline in ldebug.c (e.g., when called by luaG_traceexec) because it incorrectly expects that an oldpc value is always updated upon a return of the flow of control to a function.
CWE description: NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
CVSS v3 base score: 5.5
Published: 2020-07-24T21:15Z
Repository: https://github.com/lua/lua
Language: C
Files changed: ldebug.c, ldebug.h, ldo.c, lstate.c, lstate.h, lvm.c
Commit date: 2020-07-17 11:01:05 -0300
Commit message: Fixed bug: invalid 'oldpc' when returning to a function

The field 'L->oldpc' is not always updated when control returns to a
function; an invalid value can seg. fault when computing 'changedline'.
(One example is an error in a finalizer; control can return to
'luaV_execute' without executing 'luaD_poscall'.) Instead of trying to
fix all possible corner cases, it seems safer to be resilient to invalid
values for 'oldpc'. Valid but wrong values at most cause an extra call
to a line hook.

Diff: diff --git a/ldebug.c b/ldebug.c
index afdc2b74a..0c4439c18 100644
--- a/ldebug.c
+++ b/ldebug.c
@@ -31,14 +31,12 @@
 
 
 
 #define noLuaClosure(f)		((f) == NULL || (f)->c.tt == LUA_VCCL)
 
-
-/* Active Lua function (given call info) */
-#define ci_func(ci)		(clLvalue(s2v((ci)->func)))
-
+/* inverse of 'pcRel' */
+#define invpcRel(pc, p)		((p)->code + (pc) + 1)
 
 static const char *funcnamefromcode (lua_State *L, CallInfo *ci,
                                     const char **name);
 
 
@@ -125,24 +123,22 @@ static void settraps (CallInfo *ci) {
 
 
 /*
 ** This function can be called during a signal, under "reasonable"
 ** assumptions.
-** Fields 'oldpc', 'basehookcount', and 'hookcount' (set by
-** 'resethookcount') are for debug only, and it is no problem if they
-** get arbitrary values (causes at most one wrong hook call). 'hookmask'
-** is an atomic value. We assume that pointers are atomic too (e.g., gcc
-** ensures that for all platforms where it runs). Moreover, 'hook' is
-** always checked before being called (see 'luaD_hook').
+** Fields 'basehookcount' and 'hookcount' (set by 'resethookcount')
+** are for debug only, and it is no problem if they get arbitrary
+** values (causes at most one wrong hook call). 'hookmask' is an atomic
+** value. We assume that pointers are atomic too (e.g., gcc ensures that
+** for all platforms where it runs). Moreover, 'hook' is always checked
+** before being called (see 'luaD_hook').
 */
 LUA_API void lua_sethook (lua_State *L, lua_Hook func, int mask, int count) {
   if (func == NULL || mask == 0) {  /* turn off hooks? */
     mask = 0;
     func = NULL;
   }
-  if (isLua(L->ci))
-    L->oldpc = L->ci->u.l.savedpc;
   L->hook = func;
   L->basehookcount = count;
   resethookcount(L);
   L->hookmask = cast_byte(mask);
   if (mask)
@@ -793,14 +789,28 @@ static int changedline (const Proto *p, int oldpc, int newpc) {
   }
   return 0;  /* no line changes in the way */
 }
 
 
+/*
+** Traces the execution of a Lua function. Called before the execution
+** of each opcode, when debug is on. 'L->oldpc' stores the last
+** instruction traced, to detect line changes. When entering a new
+** function, 'npci' will be zero and will test as a new line without
+** the need for 'oldpc'; so, 'oldpc' does not need to be initialized
+** before. Some exceptional conditions may return to a function without
+** updating 'oldpc'. In that case, 'oldpc' may be invalid; if so, it is
+** reset to zero.  (A wrong but valid 'oldpc' at most causes an extra
+** call to a line hook.)
+*/
 int luaG_traceexec (lua_State *L, const Instruction *pc) {
   CallInfo *ci = L->ci;
   lu_byte mask = L->hookmask;
+  const Proto *p = ci_func(ci)->p;
   int counthook;
+  /* 'L->oldpc' may be invalid; reset it in this case */
+  int oldpc = (L->oldpc < p->sizecode) ? L->oldpc : 0;
   if (!(mask & (LUA_MASKLINE | LUA_MASKCOUNT))) {  /* no hooks? */
     ci->u.l.trap = 0;  /* don't need to stop again */
     return 0;  /* turn off 'trap' */
   }
   pc++;  /* reference is always next instruction */
@@ -817,19 +827,18 @@ int luaG_traceexec (lua_State *L, const Instruction *pc) {
   if (!isIT(*(ci->u.l.savedpc - 1)))
     L->top = ci->top;  /* prepare top */
   if (counthook)
     luaD_hook(L, LUA_HOOKCOUNT, -1, 0, 0);  /* call count hook */
   if (mask & LUA_MASKLINE) {
-    const Proto *p = ci_func(ci)->p;
     int npci = pcRel(pc, p);
     if (npci == 0 ||  /* call linehook when enter a new function, */
-        pc <= L->oldpc ||  /* when jump back (loop), or when */
-        changedline(p, pcRel(L->oldpc, p), npci)) {  /* enter new line */
+        pc <= invpcRel(oldpc, p) ||  /* when jump back (loop), or when */
+        changedline(p, oldpc, npci)) {  /* enter new line */
       int newline = luaG_getfuncline(p, npci);
       luaD_hook(L, LUA_HOOKLINE, newline, 0, 0);  /* call line hook */
     }
-    L->oldpc = pc;  /* 'pc' of last call to line hook */
+    L->oldpc = npci;  /* 'pc' of last call to line hook */
   }
   if (L->status == LUA_YIELD) {  /* did hook yield? */
     if (counthook)
       L->hookcount = 1;  /* undo decrement to zero */
     ci->u.l.savedpc--;  /* undo increment (resume will increment it again) */
diff --git a/ldebug.h b/ldebug.h
index 1fe0efab0..a0a584862 100644
--- a/ldebug.h
+++ b/ldebug.h
@@ -11,10 +11,15 @@
 #include "lstate.h"
 
 
 #define pcRel(pc, p)	(cast_int((pc) - (p)->code) - 1)
 
+
+/* Active Lua function (given call info) */
+#define ci_func(ci)		(clLvalue(s2v((ci)->func)))
+
+
 #define resethookcount(L)	(L->hookcount = L->basehookcount)
 
 /*
 ** mark for entries in 'lineinfo' array that has absolute information in
 ** 'abslineinfo' array
diff --git a/ldo.c b/ldo.c
index 4c976a148..98dd9fbbf 100644
--- a/ldo.c
+++ b/ldo.c
@@ -325,11 +325,11 @@ void luaD_hookcall (lua_State *L, CallInfo *ci) {
 
 static StkId rethook (lua_State *L, CallInfo *ci, StkId firstres, int nres) {
   ptrdiff_t oldtop = savestack(L, L->top);  /* hook may change top */
   int delta = 0;
   if (isLuacode(ci)) {
-    Proto *p = clLvalue(s2v(ci->func))->p;
+    Proto *p = ci_func(ci)->p;
     if (p->is_vararg)
       delta = ci->u.l.nextraargs + p->numparams + 1;
     if (L->top < ci->top)
       L->top = ci->top;  /* correct top to run hook */
   }
@@ -338,12 +338,12 @@ static StkId rethook (lua_State *L, CallInfo *ci, StkId firstres, int nres) {
     ci->func += delta;  /* if vararg, back to virtual 'func' */
     ftransfer = cast(unsigned short, firstres - ci->func);
     luaD_hook(L, LUA_HOOKRET, -1, ftransfer, nres);  /* call it */
     ci->func -= delta;
   }
-  if (isLua(ci->previous))
-    L->oldpc = ci->previous->u.l.savedpc;  /* update 'oldpc' */
+  if (isLua(ci = ci->previous))
+    L->oldpc = pcRel(ci->u.l.savedpc, ci_func(ci)->p);  /* update 'oldpc' */
   return restorestack(L, oldtop);
 }
 
 
 /*
diff --git a/lstate.c b/lstate.c
index 28853dc7b..06fa13d72 100644
--- a/lstate.c
+++ b/lstate.c
@@ -299,10 +299,11 @@ static void preinit_thread (lua_State *L, global_State *g) {
   L->allowhook = 1;
   resethookcount(L);
   L->openupval = NULL;
   L->status = LUA_OK;
   L->errfunc = 0;
+  L->oldpc = 0;
 }
 
 
 static void close_state (lua_State *L) {
   global_State *g = G(L);
diff --git a/lstate.h b/lstate.h
index 2e8bd6c48..0c545ec5b 100644
--- a/lstate.h
+++ b/lstate.h
@@ -284,21 +284,21 @@ struct lua_State {
   lu_byte allowhook;
   unsigned short nci;  /* number of items in 'ci' list */
   StkId top;  /* first free slot in the stack */
   global_State *l_G;
   CallInfo *ci;  /* call info for current function */
-  const Instruction *oldpc;  /* last pc traced */
   StkId stack_last;  /* last free slot in the stack */
   StkId stack;  /* stack base */
   UpVal *openupval;  /* list of open upvalues in this stack */
   GCObject *gclist;
   struct lua_State *twups;  /* list of threads with open upvalues */
   struct lua_longjmp *errorJmp;  /* current error recover point */
   CallInfo base_ci;  /* CallInfo for first level (C calling Lua) */
   volatile lua_Hook hook;
   ptrdiff_t errfunc;  /* current error handling function (stack index) */
   l_uint32 nCcalls;  /* number of allowed nested C calls - 'nci' */
+  int oldpc;  /* last pc traced */
   int stacksize;
   int basehookcount;
   int hookcount;
   volatile l_signalT hookmask;
 };
diff --git a/lvm.c b/lvm.c
index 66d451b0c..08681af1b 100644
--- a/lvm.c
+++ b/lvm.c
@@ -1792,11 +1792,11 @@ void luaV_execute (lua_State *L, CallInfo *ci) {
       }
       vmcase(OP_VARARGPREP) {
         ProtectNT(luaT_adjustvarargs(L, GETARG_A(i), ci, cl->p));
         if (trap) {
           luaD_hookcall(L, ci);
-          L->oldpc = pc + 1;  /* next opcode will be seen as a "new" line */
+          L->oldpc = 1;  /* next opcode will be seen as a "new" line */
         }
         updatebase(ci);  /* function has new base after adjustment */
         vmbreak;
       }
       vmcase(OP_EXTRAARG) {

Vulnerable code: 
/* Active Lua function (given call info) */
#define ci_func(ci)		(clLvalue(s2v((ci)->func)))

** Fields 'oldpc', 'basehookcount', and 'hookcount' (set by
** 'resethookcount') are for debug only, and it is no problem if they
** get arbitrary values (causes at most one wrong hook call). 'hookmask'
** is an atomic value. We assume that pointers are atomic too (e.g., gcc
** ensures that for all platforms where it runs). Moreover, 'hook' is
** always checked before being called (see 'luaD_hook').
  if (isLua(L->ci))
    L->oldpc = L->ci->u.l.savedpc;
    const Proto *p = ci_func(ci)->p;
        pc <= L->oldpc ||  /* when jump back (loop), or when */
        changedline(p, pcRel(L->oldpc, p), npci)) {  /* enter new line */
    L->oldpc = pc;  /* 'pc' of last call to line hook */
    Proto *p = clLvalue(s2v(ci->func))->p;
  if (isLua(ci->previous))
    L->oldpc = ci->previous->u.l.savedpc;  /* update 'oldpc' */
  const Instruction *oldpc;  /* last pc traced */
          L->oldpc = pc + 1;  /* next opcode will be seen as a "new" line */
Fixed code: /* inverse of 'pcRel' */
#define invpcRel(pc, p)		((p)->code + (pc) + 1)
** Fields 'basehookcount' and 'hookcount' (set by 'resethookcount')
** are for debug only, and it is no problem if they get arbitrary
** values (causes at most one wrong hook call). 'hookmask' is an atomic
** value. We assume that pointers are atomic too (e.g., gcc ensures that
** for all platforms where it runs). Moreover, 'hook' is always checked
** before being called (see 'luaD_hook').
/*
** Traces the execution of a Lua function. Called before the execution
** of each opcode, when debug is on. 'L->oldpc' stores the last
** instruction traced, to detect line changes. When entering a new
** function, 'npci' will be zero and will test as a new line without
** the need for 'oldpc'; so, 'oldpc' does not need to be initialized
** before. Some exceptional conditions may return to a function without
** updating 'oldpc'. In that case, 'oldpc' may be invalid; if so, it is
** reset to zero.  (A wrong but valid 'oldpc' at most causes an extra
** call to a line hook.)
*/
  const Proto *p = ci_func(ci)->p;
  /* 'L->oldpc' may be invalid; reset it in this case */
  int oldpc = (L->oldpc < p->sizecode) ? L->oldpc : 0;
        pc <= invpcRel(oldpc, p) ||  /* when jump back (loop), or when */
        changedline(p, oldpc, npci)) {  /* enter new line */
    L->oldpc = npci;  /* 'pc' of last call to line hook */

/* Active Lua function (given call info) */
#define ci_func(ci)		(clLvalue(s2v((ci)->func)))


    Proto *p = ci_func(ci)->p;
  if (isLua(ci = ci->previous))
    L->oldpc = pcRel(ci->u.l.savedpc, ci_func(ci)->p);  /* update 'oldpc' */
  L->oldpc = 0;
  int oldpc;  /* last pc traced */
          L->oldpc = 1;  /* next opcode will be seen as a "new" line */