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 When Processing Markdown Files #4

Closed
Halcy0nic opened this issue Jun 29, 2022 · 7 comments
Closed

Stack Based Buffer Overflow When Processing Markdown Files #4

Halcy0nic opened this issue Jun 29, 2022 · 7 comments

Comments

@Halcy0nic
Copy link

Hi!

I'm a big fan of md2roff. It's been quite useful and has come in handy in so many situations!

Stack Based Buffer Overflow

I wanted to make you aware of a stack based buffer overflow vulnerability in the md2roff tool. A buffer overflow condition exists when a program attempts to put more data in a buffer than it can hold or when a program attempts to put data in a memory area past a buffer. At a minimum this will lead to a denial of service (if md2roff is run as a server/service) but can also lead to arbitrary code execution and privilege escalation as a result of the return pointer (on the stack) being overwritten.

Reproduction

To reproduce the vulnerability, execute the following commands in Linux once you have compiled the program (using the default Makefile).

Create a markdown file with a large number of integers:

vuln_md_file

Verify the markdown file contains our large buffer of '1's:

overflow_file

Execute md2roff using any preferred flags and confirm the segfault:

segfault

Using GDB we can see that we successfully redirected the execution of the program. We can se our '1's on the stack and the program attempting to return to 0x3131313131313131 (which is 1 repeated in hex).

gdb_registers

gdb_return_code

Remediation

Replace all instances of strcpy with strncpy and ensure the content being read into the buffer is the same size or smaller than the available buffer space:

strcpy(d, s);

strcpy(buf, src);

strcpy(pat, mdic[i].wrong);

strcpy(appname, docname);

Useful References

https://owasp.org/www-community/vulnerabilities/Buffer_Overflow
https://cwe.mitre.org/data/definitions/121.html
https://man7.org/linux/man-pages/man3/strcpy.3.html -> Check the warning in the description
https://linux.die.net/man/3/strncpy -> Safer way to copy a buffer

@nereusx
Copy link
Owner

nereusx commented Jun 30, 2022

True it is, md2roff is a utility that designed to have no such protection. Please explain to me how this can give root privileges since it is designed to run in userspace ?

@nereusx
Copy link
Owner

nereusx commented Jun 30, 2022

btw strcpy(pat, mdic[i].wrong); <-- this is wrong since it is copy form a static array

@nereusx
Copy link
Owner

nereusx commented Jun 30, 2022

104: strcpy(d, s);
also wrong, it is strdup() and d size is = strlen(s) + 1

@nereusx
Copy link
Owner

nereusx commented Jun 30, 2022

164: strcpy(buf, src);
also wrong, buf = strlen(src) + 64KB, temporary storage of regular expression result. It could had meaning if the pattern wasnt from static array.
If it could be from user input (not a constant from static array) it could be search pattern (^.*) and replace string \1\1\1\1\1\1\1\1\1\1\1...

@nereusx
Copy link
Owner

nereusx commented Jun 30, 2022

I added limit for the headers words and phrases (titles), but this is not the only unspecified sized string.

   while ( isblank(*p) ) p ++;
    d = start_buffer;
    while ( *p ) {
        if ( isspace(*p) ) break;
        if ( d - start_buffer >= MAX_STR ) break; // check limit and break to the next word
        *d ++ = *p ++;
        }
    *d = '\0';

@Halcy0nic
Copy link
Author

Hi @nereusx,

Sorry for the confusion. I am not suggesting that all of the referenced strcpy instances are vulnerable, rather that 'strcpy' has been superseded by 'strncpy'. In reference to privilege escalation, if the binary is compiled with sudo privileges or is run as a server/service by another user, the user taking advantage of this overflow will inherit their access privileges.

  • Binary is run as a service by the user Joe -> attacker gains access to Joe's account
  • Binary is compiled/run as SUID -> attacker gains access to root account

As long as the available buffer (on either the heap or stack) is large enough to contain the input string this problem will disappear. The risk here isn't super high, but I thought I would just give you a heads up 👍

@nereusx
Copy link
Owner

nereusx commented Jun 30, 2022

Well, it is safe enough now. If you want to use it with inetd as a service, tell me to add paragraph buffer size check too.

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

2 participants