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

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

Open
thestinger opened this issue Aug 9, 2022 · 5 comments

Comments

@thestinger
Copy link

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
Copy link
Owner

Benau commented Aug 9, 2022

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

@thestinger
Copy link
Author

It think it had the same issue but it was fixed in Samsung/rlottie@7bcbea3.

@Benau
Copy link
Owner

Benau commented Aug 9, 2022

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
Copy link
Author

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
Copy link
Owner

Benau commented Aug 9, 2022

Ok will fix in few days

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