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

Stack based buffer overflow while parsing AQL (name of relation) #2486

Open
cve-reporting opened this issue Jul 9, 2018 · 0 comments
Open

Comments

@cve-reporting
Copy link

Macro AQL_ADD_RELATION write data into stack based fixed size buffer named relations without any check on number and sizes of added relations.
Buffer is declared as:
aql.h:148:
struct aql_adt {
char relations[AQL_RELATION_LIMIT][RELATION_NAME_LENGTH + 1];
..

db-options.h:128
#define AQL_RELATION_LIMIT 3
db-options.h:167
#define RELATION_NAME_LENGTH 10

usage:
aql.h:191:
#define AQL_ADD_RELATION(adt, rel)
strcpy((adt)->relations[(adt)->relation_count++], (rel))

Following AQL code samples will trigger crash:
SELECT a from a0123456789012x,b0123456789012y,c0123456789012z,d01234567890123,a0123456789012x,b0123456789012y,c0123456789012z,d01234567890123,a0123456789012x,b0123456789012y,c0123456789012z,d01234567890123,a0123456789012x,b0123456789012y,c0123456789012z,d01234567890123,a0123456789012x,b0123456789012y,c0123456789012z,d01234567890123,a0123456789012x,b0123456789012y,c0123456789012z,d01234567890123,a0123456789012x,b0123456789012y,c0123456789012z,d01234567890123

This could lead to Remote Code Execution via stack smashing attack (overwriting the function return address).
Please take a note, that before going over the whole aql_adt structure firstly other values in the struct are overwritten, which may produce other integrity issues.
The risk of this issue is reduced (Attack Vector:Local) because attacker would need to run malicious AQL query, however it is quite possible when using database in IoT application.

Proposed CVSS score:
CVSS:3.0/AV:L/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H (9.3 - critical)

Mitigation:
Before adding relation to the struct the size of new relation and the number of already stored relations should be checked.
Please take a look at patch fixing this issue in TizenRT (using antelope engine as arastorage):
https://github.com/Samsung/TizenRT/blob/master/framework/src/arastorage/aql.h
https://github.com/Samsung/TizenRT/blob/master/framework/src/arastorage/aql_adt.c

Crash details using Address Sanitizer:

=================================================================
==3393==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff5b34fa18 at pc 0x7fc11b95a709 bp 0x7fff5b34f430 sp 0x7fff5b34ebd8
WRITE of size 2 at 0x7fff5b34fa18 thread T0
#0 0x7fc11b95a708 (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x62708)
#1 0x404ff1 in strcpy /usr/include/x86_64-linux-gnu/bits/string3.h:110
#2 0x404ff1 in parse_relations contiki/apps/antelope/aql-parser.c:229
#3 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#4 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#5 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#6 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#7 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#8 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#9 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#10 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#11 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#12 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#13 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#14 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#15 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#16 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#17 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#18 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#19 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#20 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#21 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#22 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#23 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#24 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#25 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#26 0x4050af in parse_relations contiki/apps/antelope/aql-parser.c:232
#27 0x405ae7 in parse_select contiki/apps/antelope/aql-parser.c:492
#28 0x406b8f in aql_parse contiki/apps/antelope/aql-parser.c:838
#29 0x401f29 in main contiki/apps/antelope/test_aql.c:176
#30 0x7fc11b54e82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#31 0x402338 in _start (contiki/apps/antelope/test_antelope_dbg.exe+0x402338)

Address 0x7fff5b34fa18 is located in stack of thread T0 at offset 296 in frame
#0 0x401d5f in main contiki/apps/antelope/test_aql.c:138

This frame has 1 object(s):
[32, 296) 'parsed_aql' <== Memory access at offset 296 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
(longjmp and C++ exceptions are supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow ??:0 ??
Shadow bytes around the buggy address:
0x10006b661ef0: 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2 00 00
0x10006b661f00: 00 00 f2 f2 f2 f2 00 04 f4 f4 f2 f2 f2 f2 00 00
0x10006b661f10: f4 f4 f3 f3 f3 f3 00 00 00 00 00 00 00 00 f1 f1
0x10006b661f20: f1 f1 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10006b661f30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10006b661f40: 00 00 00[f4]f4 f4 f3 f3 f3 f3 00 00 00 00 00 00
0x10006b661f50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10006b661f60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10006b661f70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10006b661f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10006b661f90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
==3393==ABORTING

alexrayne pushed a commit to alexrayne/contiki that referenced this issue Nov 10, 2023
cooja: remove home-grown thread routines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant