PowerShell / PowerShell-Native

MIT License
50 stars 33 forks source link

libpsl-native: Fix _FORTIFY_SOURCE macros #88

Closed disconnect3d closed 1 year ago

disconnect3d commented 1 year ago

This commit fixes the mistake in the _FORTIFY_SOURCE macro where it was not prefixed with underscore while it has to be (see e.g. https://github.com/search?q=repo%3Abminor%2Fglibc%20FORTIFY_SOURCE&type=code).

Additionally, to make this macro add extra security, one has to enable optimizations. I am not sure if the build system enables them, but it is worth double checking that as well.

Overall, I would recommend using -D_FORTIFY_SOURCE=3 with -O2 or -O3. (The fortify source level 3 was added recently and you can read more about it here: https://developers.redhat.com/blog/2021/04/16/broadening-compiler-checks-for-buffer-overflows-in-_fortify_source).

You can also see the result of the correct vs incorrect macro along with optimizations and no optimizations on this screenshot (source): image

adityapatwardhan commented 1 year ago

@andschwa - can you help with this?

andyleejordan commented 1 year ago

@adityapatwardhan Yes, taking a look.

andyleejordan commented 1 year ago

Ok, I'm confirming that the generated build files (for a release build) are currently including:

-DFORTIFY_SOURCE=2 -O2 -O3 -DNDEBUG

so I think we should edit this PR to remove the -O2 specification, as CMake is already adding -O3 for release builds.

@disconnect3d is correct and this macro is currently doing nothing, as it needs the prepended underscore.

I like the idea of setting _FORTIFY_SOURCE=3 but with it being so new, I don't think it's worth the risk of unforeseen consequences for now.

andyleejordan commented 1 year ago

For reference @adityapatwardhan, from the CMakeCache.txt file:

//Flags used by the CXX compiler during RELEASE builds.
CMAKE_CXX_FLAGS_RELEASE:STRING=-O3 -DNDEBUG

//Flags used by the CXX compiler during RELWITHDEBINFO builds.
CMAKE_CXX_FLAGS_RELWITHDEBINFO:STRING=-O2 -g -DNDEBUG

and I confirmed with the commit I just added to @disconnect3d's branch that the generated flags are now:

CXX_FLAGSarm64 =  -std=c++11 -Wall -Werror -fstack-protector-strong -fpie -D_FORTIFY_SOURCE=2 -O3 -DNDEBUG -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk -fPIC -g -mmacosx-version-min=10.9

so no more superfluous -O2 and the macro is correctly specified.