Closed orbisvicis closed 6 years ago
refiner.h / refinerThreaded.h contains a nested function. Is there any way to get rid of this?
Uploaded a patch (compiles but untested) at http://sprunge.us/AHLG
Thanks. I don't understand why it is a security issue? I understand the potential that a virus could attempt to insert its own code on the stack so that it gets executed when the plugin calls what it thinks is it's own function code on the stack. But can't we rely on GIMP and Python to prevent a virus from altering the stack? In other words, this is secondary in the sense that there must be some other breach before this vulnerability can be exploited?
Having said that, I am open to changes. I will look at your patch.
Lets assume there exists a vulnerability, for example a stack buffer overflow, which for some input allows the stack frame pointer to be overwritten to point to custom code in the stack, usually in the same stack frame I gather. [http://en.wikipedia.org/wiki/Stack_buffer_overflow]
This vulnerability can exist in two possible locations: GIMP plug-in (Resynthesizer)
That said, being unfamiliar to the concept of an executable stack and DEP in general, this particular vulnerability having been caught by an automated package sanitation tool that my distribution [Arch Linux] provides, I wrote a small program to test the effects of dlopen() on the executable bit of the stack, uploaded at http://sprunge.us/aFNG.
An excerpt of the pertinent results is at http://hpaste.org/72363. Summarized, glibc will split the stack into two mmap'ed partitions and mark all but the highest page executable. I believe this make the entire gimp program susceptible to executable-stack-based attacks. [http://kambing.ui.ac.id/blankon/eglibc-2.13/elf/dl-execstack.c]
But can't we rely on GIMP and Python to prevent a virus from altering the stack?
If you mean, will GIMP and Python sanitize the input behaviour for plug-ins?
Hey, I'm a graphic designer and a security and privacy nerd. Hence, I'm more and more shifting towards security minded OSes such as Subgraph and Tails. As for the first, it outright blocks resynthesizer. Here's the output of sudo journalctl -f
while I'm trying to execute it:
Oct 24 00:13:06 subgraph audit[15285]: ANOM_ABEND auid=1000 uid=1000 gid=1000 ses=3 pid=15285 comm="resynthesizer" exe="/usr/lib/gimp/2.0/plug-ins/resynthesizer" sig=9
Oct 24 00:13:06 subgraph kernel: PAX: execution attempt in: <stack>, 3df80893000-3df808b5000 3fffffdd000
Oct 24 00:13:06 subgraph kernel: PAX: terminating task: /usr/lib/gimp/2.0/plug-ins/resynthesizer(resynthesizer):15286, uid/euid: 1000/1000, PC: 000003df808b215c, SP: 000003b7fc7d0568
Oct 24 00:13:06 subgraph kernel: PAX: bytes at PC: 49 bb b0 50 68 63 3e 00 00 00 49 ba 10 21 8b 80 df 03 00 00
Oct 24 00:13:06 subgraph kernel:
Oct 24 00:13:06 subgraph kernel: PAX: bytes at SP-8: 0000000000000000 0000003e636851e1 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 0000000000000000 000003df808b2070 0000000000000000 0000000000000000
Oct 24 00:13:06 subgraph kernel:
Oct 24 00:13:06 subgraph kernel: grsec: denied resource overstep by requesting 4096 for RLIMIT_CORE against limit 0 for /usr/lib/gimp/2.0/plug-ins/resynthesizer[resynthesizer:15286] uid/euid:1000/1000 gid/egid:1000/1000, parent /usr/bin/gimp-2.8[gimp:13340] uid/euid:1000/1000 gid/egid:1000/1000
There are ways to make Subgraph's security settings more permissive towards specified applications, and I've checked those with its maintainers. But, as they said, "PaX just straight up says NO to executable stacks lol" and "Most compilers default to no executable stack, and have done for probably a decade (...) Its kinda alarming you have a program that needs an executable stack to work". So, definitely a no fix for that on their part.
Such a shame. Yours is certainly one of the most useful plugins available for GIMP, I hope it gets updated someday soon.
My regards,
Yes, I intend to update it soon. Several minor patches have built up. But I am not going to release even small patches without testing, which takes time. I think I can change to not use a nested function. Not sure why it is nested in the first place.
You quote "Most compilers default to no executable stack, and have done for probably a decade" but I don't recall that I disabled any compiler options in that regard, and I use a fairly recent gcc compiler? I could be wrong, maybe the compiler did warn me about it.
As I said before, I think this is a secondary vector. As I understand it, first an attacker must overflow a data structure on the stack (to insert their executable code in the stack, and they must do it in a very hardcoded way so that it ends up at the address where the app later begins executing from the stack). So I am not very concerned: first you have to show me another exploit in my code that allows a buffer overflow on the stack. Images are not on the stack, they are on the heap. But if it holds you up, I will fix it.
I am interested in security too. But a different approach: studying Rust which purports to be safe. I don't know what the Rust language says about executing from the stack, but I think it would say that the first vector (overflow of buffer on the stack) could never happen (could not be accidentally written in any code that compiles.)
Thanks
An update, having looked again at this issue.
The reason refiner.h contains a nested function: the nested function is a callback that uses the context local variables of the function it is nested in. Said context is the progress of the application: what percentage of work has been done. The Resynthesizer, with much effort, was made reentrant (not using any global variables.) I suppose I could pass a pointer to the context to the callback function, and copy the pointer to the higher level callback function into the context (on the stack). The lower callback function is the one that calls the higher callback function, and so needs a pointer to the function. The links to the proposed patch by orbisvicis are broken, I would like to see them, or a typical way of doing this, before embarking on a fix.
Fixed by v2.0.3
Thanks bootchk, I've tested it in a sandboxed Gimp using Subgraph OS, and it worked flawlessly.
the library plugins contain an executable stack. I can't figure out why, but this is a security concern.