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

DTrace support for OS X #6

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

DTrace support for OS X #6

wants to merge 20 commits into from

Conversation

dyjakan
Copy link

@dyjakan dyjakan commented May 15, 2015

Following commits enable usage of DTrace on OS X platform as ltrace replacement.

@wapiflapi
Copy link
Owner

Thanks a lot for this work! I have some friends that where complaining about this exact problem.
I'll try it out soon and if there are no problems I'll merge it. Thanks again :)

cc @BestPig maybe you want to look at this?

@BestPig
Copy link
Contributor

BestPig commented May 15, 2015

@dyjakan why did you set the destructive mode?
It appears that you don't use destructive actions (https://wikis.oracle.com/display/DTrace/Actions+and+Subroutines#ActionsandSubroutines-DestructiveActions).

Why did you set the buffer size to 256m, it is slow at the beginning, because dtrace allocate all the buffer before starting the debug process, are they really required?

@dyjakan
Copy link
Author

dyjakan commented May 15, 2015

Destructive is not required, IIRC it was a left-over from the prototype. Removing.

Extended buffer can decrease drops. However, as I already am mentioning this in the main comment I will get rid of it as the default option. Removing.

@dyjakan
Copy link
Author

dyjakan commented May 15, 2015

@BestPig I've been thinking about your previous commit. Do we really need/want a trade off between thread-local variables and potential crash in-between malloc/calloc/etc entry and return?

Let's assume memory allocators exposed via Apple's libc are thread-safe, then we don't need to care about crashing in-between entry and return. Also, if this is the case then we could delete unnecessary predicates.

On the other hand, if memory allocators exposed via Apple's libc are not thread-safe, then the bigger issue is IMO data corectness which is lost by usage of global variables.

This is interesting.

@BestPig
Copy link
Contributor

BestPig commented May 15, 2015

I didn't thought about thread-safety.

An ugly solution to avoid problem can be to duplicate variable as global and local, so return always use the self-> one, and if it crash, the global one can be read.

Of course, if two malloc is called at the "same" time, the crashed value read in global may be wrong, but I'm not sure it possible to have more "stable" way.

@dyjakan
Copy link
Author

dyjakan commented May 15, 2015

Sorry for the confussion. I did not properly reviewed your commit.

I've read villoc.py source and original version of memtrace.d would work fine regardless of the usage of END clauses and with them it actually fails.

This is the code responsible for translating return values:

        if ret is None:
            state.errors.append("%s = <error>" % call)
        else:
            state.info.append("%s = %#x" % (call, ret))

For free()'s <void> return we have this:

def sanitize(x):
    if x is None:
        return None
    if x == "<void>":
        return 0
    return int(x, 0)

However for <error> we don't have any sanitization and villoc.py is crashing when parsing those, e.g. malloc(64) = <error>.

We can add sanitization but more elegant fix is to just go back to the initial version of memtrace.d where we do not use END clauses for constructing <error> messages. Instead we return 0x0 which is in fact the value returned upon failing by malloc(), calloc(), realloc(), reallocf(), and valloc().

However, one new issue presented itself -- if reallocf() fails then the original buffer is also freed. This should be reflected on the HTML page and IMO implemented in villoc.py not in memtrace.d.

@dyjakan
Copy link
Author

dyjakan commented May 16, 2015

@BestPig @wapiflapi I'm back with some more testing:

  • reallocf() is confirmed to be freeing the original buffer when erroring. This is caught by us without any problems.
  • ltrace on Linux also returns NULL (just 0) instead of <error> when allocation fails (output identical to memtrace.d).

So, for now memtrace.d looks good, however I've stumbled upon some problems with villoc.py when erroring at realloc() and reallocf().

@dyjakan
Copy link
Author

dyjakan commented May 16, 2015

I'm finished.

@BestPig
Copy link
Contributor

BestPig commented May 16, 2015

Handling of error in a call is not working with your method if a function segfault.
Error in your tests are handle correctly only if malloc detect something wrong and an assert doesn't pass. So the assert abord and print a message is printed, but it's not always the case.
It was the reason of printing in entry and return, because in the case of a segfault, the return is never reach and no message is print by the libc, and it will not be marked as an error.

@dyjakan
Copy link
Author

dyjakan commented May 16, 2015

Explain to me why would a malloc/calloc/etc call segfault in the middle? In what specific situation?

@wapiflapi
Copy link
Owner

Corrupted metadata due to bugs/exploitation. Which is when you would want to use villoc to see what is going on and how you could exploit the heap.

On 05/16/2015 12:21 PM, Andrzej Dyjak wrote:

Explain to me why would a malloc/calloc/etc call segfault /in the
middle/? In what specific situation?


Reply to this email directly or view it on GitHub
#6 (comment).

@dyjakan
Copy link
Author

dyjakan commented May 16, 2015

@wapiflapi True, but aren't segfaults due to heap metadata corruption happen when freeing, not when allocating? Then this should be handled by villoc.py making a case that when free() did not return with <void> then it's an issue.

@dyjakan
Copy link
Author

dyjakan commented May 16, 2015

But ofc we would somehow need to mark the free() itself (but in that free() is special -- we do not use return on free() so we can use it for flagging).

@wapiflapi
Copy link
Owner

Both cases occur. It's possible to corrupt the data in order to control the result of the next malloc for example, when done incorrectly (or caused by a bug) this can easily cause malloc & friends to crash. It is definitely a possibility and it would be sad if we couldn't see it.

@dyjakan
Copy link
Author

dyjakan commented May 16, 2015

I agree that it would be sad if we would miss things, that's why I'm still involved in this problem, and ready to solve the issues. However, I've never encountered vulnerabilities that crash inside of memory allocation functions. Could you provide some examples in the form of code or articles?

In the case of free() it's obvious why it can segfault due to heap metadata corruption (because the attacker gains control over internal algorithms responsible for linked lists or whatever data structure is used in the allocator), but I'm not sure what can be controlled by the attacker in order to crash e.g. malloc().

@wapiflapi
Copy link
Owner

It's the same with malloc(), if the attacker has control over the fastbin datastructures it's possible to trigger a segfault during malloc when it removes the chosen chunk from the fastbins. More simply it's easy to crash malloc() by triggering the different abort sanity checks in the code.

@dyjakan
Copy link
Author

dyjakan commented May 16, 2015

Could you elaborate on how exactly the attacker would control fastbins? Does it require another write4? Does it require specific heap kung-foo before binning is taking place upon free()?

@wapiflapi
Copy link
Owner

A discussion about the exploitability of malloc crashes is a little out of scope here. But this is an example of an (uncontroled) crash inside malloc() for reference:

#include <stdlib.h>
#include <string.h>
#include <stdio.h>

int main(int argc, char **argv)
{
  (void) argc, (void) argv;

  void *a, *b, *c;

  a = malloc(48);
  b = malloc(48);
  c = malloc(48);

  free(a);

  // Let's say we have an underflow on c. This
  // is far from the only way to trigger a crash.
  memset(c-128, 0xff, 0x200);

  printf("A crash in malloc will be triggered *after* this print.\n");

  malloc(48);
  malloc(48);

  return 0;
}

(tested on x86_64, libc 2.19)

@dyjakan
Copy link
Author

dyjakan commented May 19, 2015

Thanks for the example. Right now I don't have much time to focus on this merge, I will get back to it ASAP.

@wapiflapi
Copy link
Owner

No rush :) I'm busy as well.

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

Successfully merging this pull request may close these issues.

3 participants