Re: Crash Analysis: Finalizer Logic in singlestep function can lead to Sandbox Escape Exploit

[김지회 <pascal4847@...>]

> As a rule, we should run finalizers only after everything else we
> have to do in the GC.

Thank you for nice guidance.
I've tried your idea, adding another argument into singlestep function 
and runtilstate function.
It seems that the patch can handle all the crashes we found.
However, I'm not good at interpreter implementation. There may be some 
side-effects in my patch.
I hope that a better patch will be accepted in the next update.

By the way, I have some questions.

1. singlestep function in incstep function
  My patch call the singlestep in incstep function with non-finalizer 
flag, otherwise crash occur in example ofcrash2.lua
  But I can't be sure that the patch is right.

2. gcstate after non-finalizer GCScallfin
  I've set gcstate into GCSpause, but is it the correct way?

The following log is my custom hotfix to handle the crash.

[diff patch/lgc.h origin/lgc.h]
177,179d176
< #define GC_NOFIN    0
< #define GC_YESFIN    1
<
183c180
< LUAI_FUNC void luaC_runtilstate (lua_State *L, int statesmask, int 
finalizerflag);
---
> LUAI_FUNC void luaC_runtilstate (lua_State *L, int statesmask);


[diff patch/lgc.c origin/lgc.c]

1295,1296c1295,1296
<   luaC_runtilstate(L, bitmask(GCSpause), GC_NOFIN);  /* prepare to 
start a new cycle */
<   luaC_runtilstate(L, bitmask(GCSpropagate), GC_NOFIN);  /* start new 
cycle */
---
>   luaC_runtilstate(L, bitmask(GCSpause));  /* prepare to start a new 
cycle */
>   luaC_runtilstate(L, bitmask(GCSpropagate));  /* start new cycle */

1379c1379
<   luaC_runtilstate(L, bitmask(GCSpropagate), GC_NOFIN);  /* start new 
cycle */
---
>   luaC_runtilstate(L, bitmask(GCSpropagate));  /* start new cycle */

1388c1388
<     luaC_runtilstate(L, bitmask(GCSpause), GC_YESFIN);  /* finish 
collection */
---
>     luaC_runtilstate(L, bitmask(GCSpause));  /* finish collection */

1576c1576
< static lu_mem singlestep (lua_State *L, int finalizerflag) {
---
> static lu_mem singlestep (lua_State *L) {

1624,1628c1624
<         g->gcstate = GCSpause;
<         work = 0;
<         if(finalizerflag == GC_YESFIN){
<                               work = runafewfinalizers(L, GCFINMAX) * 
GCFINALIZECOST;
<         }
---
>         work = runafewfinalizers(L, GCFINMAX) * GCFINALIZECOST;

1647c1643
< void luaC_runtilstate (lua_State *L, int statesmask, int finalizerflag) {
---
> void luaC_runtilstate (lua_State *L, int statesmask) {

1650c1646
<     singlestep(L, finalizerflag);
---
>     singlestep(L);

1668c1664
<     lu_mem work = singlestep(L, GC_NOFIN);  /* perform one single step */
---
>     lu_mem work = singlestep(L);  /* perform one single step */

1705,1706c1701,1702
<   luaC_runtilstate(L, bitmask(GCSpause), GC_NOFIN);
<   luaC_runtilstate(L, bitmask(GCScallfin), GC_NOFIN);  /* run up to 
finalizers */
---
>   luaC_runtilstate(L, bitmask(GCSpause));
>   luaC_runtilstate(L, bitmask(GCScallfin));  /* run up to finalizers */

1709c1705
<   luaC_runtilstate(L, bitmask(GCSpause), GC_YESFIN);  /* finish 
collection */
---
>   luaC_runtilstate(L, bitmask(GCSpause));  /* finish collection */




--Regards, Jihoi.