Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cp->packstack buffer overflow in cp_pragma() #1114

Closed
Buristan opened this issue Nov 7, 2023 · 1 comment
Closed

cp->packstack buffer overflow in cp_pragma() #1114

Buristan opened this issue Nov 7, 2023 · 1 comment

Comments

@Buristan
Copy link

Buristan commented Nov 7, 2023

Originally reported via tarantool/tarantool#9339:
cp->packstack is of size CPARSE_MAX_PACKSTACK (7).
cp->curpack is checked to be less than CPARSE_MAX_PACKSTACK, but then cp->packstack is accessed at cp->curpack + 1, which is out of bounds, so cp->curpack value is overwritten.

The following example demonstrates the issue:

src/luajit -e "
local ffi = require 'ffi'
ffi.cdef[[
#pragma pack(push, 1)
#pragma pack(push, 1)
#pragma pack(push, 1)
#pragma pack(push, 1)
#pragma pack(push, 8)
#pragma pack(push, 8)
#pragma pack(push, 8)
#pragma pack(pop)
struct test {uint64_t a; uint8_t b;};
]]

-- Expected 16, got 9.
print(ffi.sizeof(ffi.new('struct test')))
"
9

For the reference, the C example (named /tmp/t.c):

#include <stdio.h>
#include <stdint.h>

#pragma pack(push, 1)
#pragma pack(push, 1)
#pragma pack(push, 1)
#pragma pack(push, 1)
#pragma pack(push, 8)
#pragma pack(push, 8)
#pragma pack(push, 8)
#pragma pack(pop)

struct test {uint64_t a; uint8_t b;};

int main(void)
{
        printf("%lu\n", sizeof(struct test));
        return 0;
}

yields as expected 16:

gcc -O0 -ggdb3 -g /tmp/t.c -o /tmp/t.exe && /tmp/t.exe
16

The following trivial patch fixes the issue (since the previous value is taken):

diff --git a/src/lj_cparse.c b/src/lj_cparse.c
index 6c3bb2f9..7963590d 100644
--- a/src/lj_cparse.c
+++ b/src/lj_cparse.c
@@ -1766,7 +1766,7 @@ static void cp_pragma(CPState *cp, BCLine pragmaline)
     cp_check(cp, '(');
     if (cp->tok == CTOK_IDENT) {
       if (cp_str_is(cp->str, "push")) {
-        if (cp->curpack < CPARSE_MAX_PACKSTACK) {
+        if (cp->curpack + 1 < CPARSE_MAX_PACKSTACK) {
           cp->packstack[cp->curpack+1] = cp->packstack[cp->curpack];
           cp->curpack++;
         }

But maybe it's better to throw an error in the case of the curpack overflow (since the behaviour when we overwrite the top stack slot value isn't very obvious)?

MikePall pushed a commit that referenced this issue Nov 7, 2023
@MikePall
Copy link
Member

MikePall commented Nov 7, 2023

Fixed. Thanks!

@MikePall MikePall closed this as completed Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants