-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Remote heap feng shui / heap spraying protection #14304
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hesitating on this one, as I feel that how zones are used belongs to upper abstraction levels (that's also why I've defined ZEND_MM_ZONE_INPUT elsewhere). In any case I agree that a comment would help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair, but I also agree that a comment would be nice :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2M * 100 workers = waster of 200MB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This consumes an additional 2MiB of VMA per worker, but I expect the overhead of actually committed memory to be much less than that in practice, as not all pages of the extra chunk are touched.
For example, at the end of a symfony-demo request, we have this:
base: VmRSS: 65772 kB
mm-zones: VmRSS: 66020 kB
Diff: 249 kB
With USE_ZEND_ALLOC_HUGE_PAGES=1, the RSS actually increases by 2MiB, but this is something to expect with huge pages, and this is not enabled by default.
Now that #14054 was merged, can you please rebase this one, so we can get it landed?
@arnaud-lb ping :)
Sorry I didn't see your previous comment. I will get back at this PR soon.
0d05928 to
ee191aa
Compare
ee191aa to
b9f761d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this lands, please add a note to UPGRADING in other changes about the increase in memory requirements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the first look, I don't like this. What kind of attacks will this complicate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2M * 100 workers = waster of 200MB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This make regression for each deallocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds an additional fetch of chunk->zone_free_slot, but this field is just next to chunk->heap, and is likely in cache as we fetch it here:
Lines 2701 to 2702 in 53df3ae
Also, benchmark results show very little overhead (0% wall time, 0.04% valgrind)
What kind of attacks will this complicate?
An attacker can precisely control the layout of the heap by sending GET or POST parameters. For example, the query string ?a=aaa...&b=bbb...&c=ccc...&b= allocates 4 blocks of arbitrary size, frees the second one, and puts it at the top of the freelist. So the attacker is able to precisely control what is allocated where during application execution, and what is next to it in memory. This greatly facilitates the exploitation of all kinds of memory safety issues. See https://www.youtube.com/watch?v=-FXvUe0tySM&t=1233s (starting at 18:28) for example.
By isolating the user inputs in separate chunks, we prevent grooming the application heap via GET/POST/etc. This is useful in the context of a remote attacker with no ability to execute arbitrary code.
As the overhead is very small (0% wall time / 0.04% valgrind on the symfony-demo benchmark), I believe this is worth it.
@arnaud-lb didn't you already add the protection against heap buffer overflow/underflow through the shadow pointers?
I thought, it shouldn't be possible to corrupt the free-list now.
why do we need this patch on top of shadow pointers?
do you have the source of the exploit used in the presentation?
Personally, I think that the better approach would be filtering input data and just stopping the request in case of unexpected/dangerous data detection (e.g. GET/POST with duplicate names).
didn't you already add the protection against heap buffer overflow/underflow through the shadow pointers?
Yes, but that's not the only way to exploit an out of bound write. If an attacker can arrange the heap to override the first bytes of an arbitrary block, there are other things they can attack. Protecting against freelist corruption specifically was worth it because it's an easy and powerful target, but there are other targets that are made practicable by arranging the layout of the heap.
do you have the source of the exploit used in the presentation?
I've tried to find it, but without success
Personally, I think that the better approach would be filtering input data and just stopping the request in case of unexpected/dangerous data detection (e.g. GET/POST with duplicate names).
This will prevent controlling the order of elements in the freelist, but this is not absolutely necessary to control block placement in the heap. For instance, an attacker can control the order of runs in a chunk (without unsetting a GET/POST element), as well as how much they are filled, so they can arrange for a block of size N allocated by the application to be at the end of a run, and just before an other run of their choice.
do you have the source of the exploit used in the presentation?
I've tried to find it, but without success
@cfreal could you please share the sources of exploit used in your presentation? (better to email to my and @arnaud-lb public github email addresses)
cfreal
commented
Nov 11, 2024
Future scope would be to activate the input zone in more places (e.g. when parsing json, during unserialize, etc), or to create more zones for various purposes.
Also, do not abuse the userinput zone, as it may introduce a new attack surface. For example, if there is a potential vulnerability in the unserialize process and unserialize-related operations are added to the userinput zone, an attacker could use an HTTP request to arrange an ideal memory layout without needing to account for PHP internals.
Also, do not abuse the userinput zone, as it may introduce a new attack surface. For example, if there is a potential vulnerability in the unserialize process and unserialize-related operations are added to the userinput zone, an attacker could use an HTTP request to arrange an ideal memory layout without needing to account for PHP internals.
Unserialized is already providing enough control to an attacker, this wouldn't change much :D But more seriously, partitioning memory by types (whether primitive types/size, or usage-types) is part of #14083's roadmap :)
@dstogov did you have time to take a look at this?
@arnaud-lb please discuss this with @nielsdos, @iluuu1994 and take the decision.
I don't like this over-complication, but I also don't like to be a blocker.
Hey @nielsdos and @iluuu1994, did you have time to look at this?
No I didn't look at this yet in detail.
I need to make time for this in the weekend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite see where the initial call for zend_mm_userinput_begin is?
@nielsdos it happens in shutdown_memory_manager(full_shutdown: false), which is called before every request
I see. I'm neutral to it, I like the protection but it is also limited in scope and doubles the minimum (削除) chunk size (削除ここまで) allocated memory (although not by a lot and it could be overcommitted).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is_zend_ptr() looks like it will also need adjustments to loop through all zones.
The concept makes sense to me. Though I'd expect quite a lot of string operations can move user-controlled inputs to the default zone.
Isolate request environment / input in separate chunks to makes it more difficult to remotely control the layout of the heap.
AWS x86_64 (c7i.24xl)
Laravel 12.2.0 demo app - 100 consecutive runs, 50 warmups, 100 requests (sec)
Symfony 2.7.0 demo app - 100 consecutive runs, 50 warmups, 100 requests (sec)
Wordpress 6.2 main page - 100 consecutive runs, 20 warmups, 20 requests (sec)
bench.php - 100 consecutive runs, 10 warmups, 2 requests (sec)
|
ff21aab to
5e7fb2c
Compare
AWS x86_64 (c7i.24xl)
Laravel 12.2.0 demo app - 100 consecutive runs, 50 warmups, 100 requests (sec)
Symfony 2.7.0 demo app - 100 consecutive runs, 50 warmups, 100 requests (sec)
Wordpress 6.2 main page - 100 consecutive runs, 20 warmups, 20 requests (sec)
bench.php - 100 consecutive runs, 10 warmups, 2 requests (sec)
|
AWS x86_64 (c7i.24xl)
Laravel 12.2.0 demo app - 100 consecutive runs, 50 warmups, 100 requests (sec)
Symfony 2.7.0 demo app - 100 consecutive runs, 50 warmups, 100 requests (sec)
Wordpress 6.2 main page - 100 consecutive runs, 20 warmups, 20 requests (sec)
bench.php - 100 consecutive runs, 10 warmups, 2 requests (sec)
|
Uh oh!
There was an error while loading. Please reload this page.
This isolates remotely-controlled allocations (user input / request environment) in separate chunks, to make it more difficult for a remote attacker to precisely control the layout of the heap before the application starts, or to spray the heap with specific content. The general idea is similar to https://lwn.net/Articles/965837/ (not necessarily the implementation).
Design:
The heap is split in two zones, each with its own set of freelists (free_slot) and chunks so that allocations in a zone do not impact the layout of the other one. There is a notion of current zone, from which all allocations are made. The current zone is switched to the input zone before handling a request, and switched back to the default zone after input handling.
A new field
heap.zone_free_slotpoints to the freelist of the current zone. The only change required in allocation operations is to refer to that instead ofheap.free_slot. Similarly, in free operations we find thezone_free_slotin the chunk.realloc() allocates new blocks in the current zone (if truncation or extension is required), regardless of the original zone in which the block was allocated, so that it's not possible to profit of the layout of the original zone.
It is valid to switch zones at any time, so we can switch zones before/after handling JIT super globals, for instance.
Future scope would be to activate the input zone in more places (e.g. when parsing json, during unserialize, etc), or to create more zones for various purposes.
Time overhead is very small (~+0% wall time, +0.13% valgrind).
The minimal memory usage is now 4MiB (two chunks) instead of 2MiB, but this does not necessarily translates to that much increase in RSS.
Related: #14083