Stack buffer overflow in genmkvpwd due to excessive max_len parameter
Nobody has claimed this yet.
- Dominant language
- C
- Stars
- 13.6k
- Forks
- 2.6k
- PR merge metrics
- No merged PRs in 30d
Description
Hi, we have found a stack buffer overflow and would like to report this issue.
We're running the proposed fix locally but haven't tested thoroughly. Please make any needed corrections and push.
Summary
When max_len > MAX_MKV_LEN, invalid memory access occurs.
Reproduction
- Operating System: Ubuntu 24.04 LTS
- Architecture: x86_64
- Compiler: GCC 14.2.0
Reproduction Steps
git clone https://github.com/openwall/john
cd john/src
./configure --enable-asan
make
../run/genmkvpwd ../run/stats -1
Output
allocated 17408 KB for nbparts
70 G possible passwords (70399740865)
=================================================================
==1042409==ERROR: AddressSanitizer: unknown-crash on address 0x7f8213d0002c at pc 0x5807372ab9f3 bp 0x7ffc9e8c2f80 sp 0x7ffc9e8c2f70
WRITE of size 34 at 0x7f8213d0002c thread T0
#0 0x5807372ab9f2 in memset /usr/include/x86_64-linux-gnu/bits/string_fortified.h:59
#1 0x5807372ab9f2 in main /tmp/john/src/genmkvpwd.c:294
#2 0x7f8215c2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#3 0x7f8215c2a28a in __libc_start_main_impl ../csu/libc-start.c:360
#4 0x5807372a97e4 in _start (/tmp/john/run/genmkvpwd+0x47e4) (BuildId: b4e075dafb2127c3f70d28cd97c5c1dcc7034212)
Address 0x7f8213d0002c is located in stack of thread T0 at offset 44 in frame
#0 0x5807372aab71 in main /tmp/john/src/genmkvpwd.c:200
This frame has 2 object(s):
[32, 76) 'pwd' (line 201) <== Memory access at offset 44 partially overflows this variable
[112, 156) 'pwd2' (line 202)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: unknown-crash /usr/include/x86_64-linux-gnu/bits/string_fortified.h:59 in memset
Shadow bytes around the buggy address:
0x7f8213cffd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7f8213cffe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7f8213cffe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7f8213cfff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7f8213cfff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x7f8213d00000: f1 f1 f1 f1 00[00]00 00 00 04 f2 f2 f2 f2 00 00
0x7f8213d00080: 00 00 00 00 00 04 f3 f3 f3 f3 00 00 00 00 00 00
0x7f8213d00100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7f8213d00180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7f8213d00200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x7f8213d00280: 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
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
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
Left alloca redzone: ca
Right alloca redzone: cb
==1042409==ABORTING
Root Cause Analysis
[src/genmkvpwd.c:294](https://github.com/openwall/john/blob/5baa3c47dbef1466b106a5894e3096b13a8c22f1/src/genmkvpwd.c#L294)
When the value of max_len exceeds the length of pwd.password, it causes invalid memory access.
[src/mkvlib.h:17](https://github.com/openwall/john/blob/5baa3c47dbef1466b106a5894e3096b13a8c22f1/src/mkvlib.h#L17)
The size of s_pwd.password is MAX_MKV_LEN + 1.
[src/params.h:482](https://github.com/openwall/john/blob/5baa3c47dbef1466b106a5894e3096b13a8c22f1/src/params.h#L482)
Reference: MAX_MKV_LEN is 30.
Proposed Fix
Adding a check similar to MAX_MKV_LVL can avoid this issue.
Using strtol instead of atoi for parsing max_len would be safer.
diff --git a/src/genmkvpwd.c b/src/genmkvpwd.c
index de62d0330..59ce93f91 100644
--- a/src/genmkvpwd.c
+++ b/src/genmkvpwd.c
@@ -272,6 +272,10 @@ int main(int argc, char * * argv)
}
nbparts = mem_alloc(256*(max_lvl+1)*sizeof(long long)*(max_len+1));
+ if (max_len > MAX_MKV_LEN){
+ fprintf(stderr, "Warning: max_len = %d is too large (max = %d)\n", max_len, MAX_MKV_LEN);
+ max_len = MAX_MKV_LEN;
+ }
fprintf(stderr, "allocated %lu KB for nbparts\n", (unsigned long)(256UL*(max_lvl+1)*(max_len+1)*sizeof(long long)/1024));
memset(nbparts, 0, 256*(max_lvl+1)*(max_len+1)*sizeof(long long));
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/genmkvpwd.c at the max_len parsing and memset around line 294, then review the buffer definition in src/mkvlib.h and MAX_MKV_LEN in src/params.h. Build with --enable-asan and run the reported genmkvpwd command; done means excessive max_len input no longer causes an invalid memory access and the existing behavior remains valid.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- cli, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 70/100