Skip to content

Commit f623b96

Browse files
committed
Bug: read overflow in 'l_strcmp'
Equality according to 'strcoll' does not imply that strings have the same length.
1 parent 9be74cc commit f623b96

File tree

1 file changed

+20
-18
lines changed

1 file changed

+20
-18
lines changed

lvm.c

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -366,30 +366,32 @@ void luaV_finishset (lua_State *L, const TValue *t, TValue *key,
366366

367367

368368
/*
369-
** Compare two strings 'ls' x 'rs', returning an integer less-equal-
370-
** -greater than zero if 'ls' is less-equal-greater than 'rs'.
369+
** Compare two strings 'ts1' x 'ts2', returning an integer less-equal-
370+
** -greater than zero if 'ts1' is less-equal-greater than 'ts2'.
371371
** The code is a little tricky because it allows '\0' in the strings
372-
** and it uses 'strcoll' (to respect locales) for each segments
373-
** of the strings.
372+
** and it uses 'strcoll' (to respect locales) for each segment
373+
** of the strings. Note that segments can compare equal but still
374+
** have different lengths.
374375
*/
375-
static int l_strcmp (const TString *ls, const TString *rs) {
376-
const char *l = getstr(ls);
377-
size_t ll = tsslen(ls);
378-
const char *r = getstr(rs);
379-
size_t lr = tsslen(rs);
376+
static int l_strcmp (const TString *ts1, const TString *ts2) {
377+
const char *s1 = getstr(ts1);
378+
size_t rl1 = tsslen(ts1); /* real length */
379+
const char *s2 = getstr(ts2);
380+
size_t rl2 = tsslen(ts2);
380381
for (;;) { /* for each segment */
381-
int temp = strcoll(l, r);
382+
int temp = strcoll(s1, s2);
382383
if (temp != 0) /* not equal? */
383384
return temp; /* done */
384385
else { /* strings are equal up to a '\0' */
385-
size_t len = strlen(l); /* index of first '\0' in both strings */
386-
if (len == lr) /* 'rs' is finished? */
387-
return (len == ll) ? 0 : 1; /* check 'ls' */
388-
else if (len == ll) /* 'ls' is finished? */
389-
return -1; /* 'ls' is less than 'rs' ('rs' is not finished) */
390-
/* both strings longer than 'len'; go on comparing after the '\0' */
391-
len++;
392-
l += len; ll -= len; r += len; lr -= len;
386+
size_t zl1 = strlen(s1); /* index of first '\0' in 's1' */
387+
size_t zl2 = strlen(s2); /* index of first '\0' in 's2' */
388+
if (zl2 == rl2) /* 's2' is finished? */
389+
return (zl1 == rl1) ? 0 : 1; /* check 's1' */
390+
else if (zl1 == rl1) /* 's1' is finished? */
391+
return -1; /* 's1' is less than 's2' ('s2' is not finished) */
392+
/* both strings longer than 'zl'; go on comparing after the '\0' */
393+
zl1++; zl2++;
394+
s1 += zl1; rl1 -= zl1; s2 += zl2; rl2 -= zl2;
393395
}
394396
}
395397
}

0 commit comments

Comments
 (0)