search

Benau / go_rlottie

MIT License
1 stars 0 forks source link

vector_pixman_pixman-arm-neon-asm.S is included on all platforms resulting in an executable stack outside ARM #1

Open thestinger opened 2 years ago

thestinger commented 2 years ago

The snippet of code in this file disabling executable stacks isn't included on non-ARM architectures and results in the stack being made executable for the entire executable where this library is included:

#if defined(__linux__) && defined(__ELF__)
.section .note.GNU-stack,"",%progbits
#endif

A trivial fix for this is moving this block of code outside of the architecture ifdef wrapped around the file.

I noticed that matterbridge had an executable stack and quickly narrowed it down to this library. The executable stack makes memory corruption bugs much more easily exploitable and deters further hardening.

Benau commented 2 years ago

This file seems to be generated from the original rlottie repo. Let me see if latest rlottie has this issue...

thestinger commented 2 years ago

It think it had the same issue but it was fixed in https://github.com/Samsung/rlottie/commit/7bcbea3a5038e054a464153c8ebdb2e22336226d.

Benau commented 2 years ago

actually why non arm should include those line in the first place? no asm code is used for non arm.

Or do you think somewhere in the cpp file has bad code?

Sorry this project is mainly a wrapper of rlottie...

thestinger commented 2 years ago

It's currently equivalent to an empty file outside ARM. The problem is that an empty assembly file has no .note.GNU-stack marker and therefore marks the stack executable. You can either change it so that it's not included like the upstream rlottie did or you can move the .note.GNU-stack part outside of the ifdef for ARM. It would be cleanest to stop including the file outside ARM as they did. They also removed that ifdef because it makes far more sense for it to be an ARM specific file instead of a file included everywhere wrapped in an ARM ifdef.

The pax-utils package on Linux has a scanelf utility that you can use to confirm in an executable including this library:

% scanelf -e /usr/bin/matterbridge
 TYPE   STK/REL/PTL FILE
ET_DYN RWX R-- RW- /usr/bin/matterbridge

It will show RWX for the stack. You can also see it at runtime with grep rwx /proc/PID/maps for the PID of the process using it.

Benau commented 2 years ago

Ok will fix in few days