Skip to content

Commit 303f415

Browse files
roberto-ieruroberto-ieru
committed
Randomness added to table length computation
A bad actor could fill only a few entries in a table (power of twos in decreasing order, see tests) and produce a small table with a huge length. If your program builds a table with external data and iterates over its length, this behavior could be an issue.
1 parent ccb8b30 commit 303f415

6 files changed

Lines changed: 48 additions & 23 deletions

File tree

‎lapi.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ LUA_API lua_Unsigned lua_rawlen (lua_State *L, int idx) {
440440
case LUA_VSHRSTR: return cast(lua_Unsigned, tsvalue(o)->shrlen);
441441
case LUA_VLNGSTR: return cast(lua_Unsigned, tsvalue(o)->u.lnglen);
442442
case LUA_VUSERDATA: return cast(lua_Unsigned, uvalue(o)->len);
443-
case LUA_VTABLE: return luaH_getn(hvalue(o));
443+
case LUA_VTABLE: return luaH_getn(L, hvalue(o));
444444
default: return 0;
445445
}
446446
}

‎lobject.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@
3131

3232

3333
/*
34-
** Computes ceil(log2(x))
34+
** Computes ceil(log2(x)), which is the smallest integer n such that
35+
** x <= (1 << n).
3536
*/
3637
lu_byte luaO_ceillog2 (unsigned int x) {
3738
static const lu_byte log_2[256] = { /* log_2[i - 1] = ceil(log2(i)) */

‎ltable.c‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,32 +1220,44 @@ void luaH_setint (lua_State *L, Table *t, lua_Integer key, TValue *value) {
12201220

12211221
/*
12221222
** Try to find a boundary in the hash part of table 't'. From the
1223-
** caller, we know that 'j' is zero or present and that 'j + 1' is
1224-
** present. We want to find a larger key that is absent from the
1225-
** table, so that we can do a binary search between the two keys to
1226-
** find a boundary. We keep doubling 'j' until we get an absent index.
1227-
** If the doubling would overflow, we try LUA_MAXINTEGER. If it is
1228-
** absent, we are ready for the binary search. ('j', being max integer,
1229-
** is larger or equal to 'i', but it cannot be equal because it is
1230-
** absent while 'i' is present; so 'j > i'.) Otherwise, 'j' is a
1231-
** boundary. ('j + 1' cannot be a present integer key because it is
1232-
** not a valid integer in Lua.)
1223+
** caller, we know that 'asize + 1' is present. We want to find a larger
1224+
** key that is absent from the table, so that we can do a binary search
1225+
** between the two keys to find a boundary. We keep doubling 'j' until
1226+
** we get an absent index. If the doubling would overflow, we try
1227+
** LUA_MAXINTEGER. If it is absent, we are ready for the binary search.
1228+
** ('j', being max integer, is larger or equal to 'i', but it cannot be
1229+
** equal because it is absent while 'i' is present.) Otherwise, 'j' is a
1230+
** boundary. ('j + 1' cannot be a present integer key because it is not
1231+
** a valid integer in Lua.)
1232+
** About 'rnd': If we used a fixed algorithm, a bad actor could fill
1233+
** a table with only the keys that would be probed, in such a way that
1234+
** a small table could result in a huge length. To avoid that, we use
1235+
** the state's seed as a source of randomness. For the first probe,
1236+
** we "randomly double" 'i' by adding to it a random number roughly its
1237+
** width.
12331238
*/
1234-
static lua_Unsigned hash_search (Table *t, lua_Unsigned j) {
1235-
lua_Unsigned i;
1236-
if (j == 0) j++; /* the caller ensures 'j + 1' is present */
1237-
do {
1239+
static lua_Unsigned hash_search (lua_State *L, Table *t, unsigned asize) {
1240+
lua_Unsigned i = asize + 1; /* caller ensures t[i] is present */
1241+
unsigned rnd = G(L)->seed;
1242+
int n = (asize > 0) ? luaO_ceillog2(asize) : 0; /* width of 'asize' */
1243+
unsigned mask = (1u << n) - 1; /* 11...111 with the width of 'asize' */
1244+
unsigned incr = (rnd & mask) + 1; /* first increment (at least 1) */
1245+
lua_Unsigned j = (incr <= l_castS2U(LUA_MAXINTEGER) - i) ? i + incr : i + 1;
1246+
rnd >>= n; /* used 'n' bits from 'rnd' */
1247+
while (!hashkeyisempty(t, j)) { /* repeat until an absent t[j] */
12381248
i = j; /* 'i' is a present index */
1239-
if (j <= l_castS2U(LUA_MAXINTEGER) / 2)
1240-
j *= 2;
1249+
if (j <= l_castS2U(LUA_MAXINTEGER)/2 - 1) {
1250+
j = j*2 + (rnd & 1); /* try again with 2j or 2j+1 */
1251+
rnd >>= 1;
1252+
}
12411253
else {
12421254
j = LUA_MAXINTEGER;
12431255
if (hashkeyisempty(t, j)) /* t[j] not present? */
12441256
break; /* 'j' now is an absent index */
12451257
else /* weird case */
12461258
return j; /* well, max integer is a boundary... */
12471259
}
1248-
} while (!hashkeyisempty(t, j)); /* repeat until an absent t[j] */
1260+
}
12491261
/* i < j && t[i] present && t[j] absent */
12501262
while (j - i > 1u) { /* do a binary search between them */
12511263
lua_Unsigned m = (i + j) / 2;
@@ -1286,7 +1298,7 @@ static lua_Unsigned newhint (Table *t, unsigned hint) {
12861298
** If there is no array part, or its last element is non empty, the
12871299
** border may be in the hash part.
12881300
*/
1289-
lua_Unsigned luaH_getn (Table *t) {
1301+
lua_Unsigned luaH_getn (lua_State *L, Table *t) {
12901302
unsigned asize = t->asize;
12911303
if (asize > 0) { /* is there an array part? */
12921304
const unsigned maxvicinity = 4;
@@ -1327,7 +1339,7 @@ lua_Unsigned luaH_getn (Table *t) {
13271339
if (isdummy(t) || hashkeyisempty(t, asize + 1))
13281340
return asize; /* 'asize + 1' is empty */
13291341
else /* 'asize + 1' is also non empty */
1330-
return hash_search(t, asize);
1342+
return hash_search(L, t, asize);
13311343
}
13321344

13331345

‎ltable.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ LUAI_FUNC void luaH_resizearray (lua_State *L, Table *t, unsigned nasize);
173173
LUAI_FUNC lu_mem luaH_size (Table *t);
174174
LUAI_FUNC void luaH_free (lua_State *L, Table *t);
175175
LUAI_FUNC int luaH_next (lua_State *L, Table *t, StkId key);
176-
LUAI_FUNC lua_Unsigned luaH_getn (Table *t);
176+
LUAI_FUNC lua_Unsigned luaH_getn (lua_State *L, Table *t);
177177

178178

179179
#if defined(LUA_DEBUG)

‎lvm.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,7 @@ void luaV_objlen (lua_State *L, StkId ra, const TValue *rb) {
722722
Table *h = hvalue(rb);
723723
tm = fasttm(L, h->metatable, TM_LEN);
724724
if (tm) break; /* metamethod? break switch to call it */
725-
setivalue(s2v(ra), l_castU2S(luaH_getn(h))); /* else primitive len */
725+
setivalue(s2v(ra), l_castU2S(luaH_getn(L, h))); /* else primitive len */
726726
return;
727727
}
728728
case LUA_VSHRSTR: {

‎testes/nextvar.lua‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,18 @@ do
345345
end
346346
end
347347

348+
349+
do print("testing attack on table length")
350+
local t = {}
351+
local lim = math.floor(math.log(math.maxinteger, 2)) - 1
352+
for i = lim, 0, -1 do
353+
t[2^i] = true
354+
end
355+
assert(t[1 << lim])
356+
-- next loop should not take forever
357+
for i = 1, #t do end
358+
end
359+
348360
local nofind = {}
349361

350362

0 commit comments

Comments
 (0)