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

GCC's -fstack-protector fails to guard dynamic stack allocations on Aarch64 #234

Closed
thomasnyman opened this issue Sep 13, 2023 · 7 comments · Fixed by #235
Closed

GCC's -fstack-protector fails to guard dynamic stack allocations on Aarch64 #234

thomasnyman opened this issue Sep 13, 2023 · 7 comments · Fixed by #235

Comments

@thomasnyman
Copy link
Contributor

On September 12th, 2023 Meta's Read Team disclosed a vulnerability in GCC on AArch64 targets tracked as CVE-2023-40339 that causes GCC's stack smashing protection not to detect or defend against overflows of dynamically-sized local variables such as variable-length arrays or buffers allocated using alloca().

At the time of disclosure, all version of GCC from 5.4.0 to trunk as of 2023-05-15 were affected.

In response, Arm issued an advisory containing a patch that has been backported to GCC 7 - 13 and are incorporated to new releases of Arm GNU Toolchain.

Given that the Compiler Options Hardening Guide for C and C++ Guide recommends the use of GCC's stack smashing protection the guide should address this to ensure readers on Aarch64 targets benefit from stack smashing protection using patched version of GCC.

@ware
Copy link

ware commented Sep 14, 2023

Are we trying to make sure developers know about all vulnerabilities in gcc? If so, why are we stopping with this one just because it's new? If not, why aren't we just telling people to use the latest, supported releases of GCC?

@david-a-wheeler
Copy link
Contributor

@ware - I think what's different is that (1) even using the latest supported release of upstream GCC won't fix the problem, and that (2) it's a huge bug specifically in its protection mechanism. Even when the fix is released, it'll take a while for this fix to propagate.

I think encouraging the use of the latest releases of compilers is in theory a good idea, though I think many developers of embedded systems would laugh and wonder when they'll be able to update to a compiler version that's only several years out of date :-(.

There's no point in noting all bugs, but this one seems different and worth noting.

@ware
Copy link

ware commented Sep 14, 2023

I get that but I also don't how long it will be relevant. You say "even when the fix is released" but it is released and already actively being integrated into Linux distributions. I mean, I'll defer but Debian has it fixed in gcc-11, gcc-12 & gcc-13. Yes, still not fixed in previous releases.

I get that this is in a security feature and that changes the profile, but security feature or not, if someone is using an older compiler, they can be vulnerable to any of these. For example, CVE-2018-12886 is also about a bug in -fstack-protector on ARM. I know there's embedded people using those affected compilers still. Should we be warning them of that?

Again, I'll defer as I wasn't in the conversation that generated this issue, but the decision to put this in feels arbitrary.

@edelsohn
Copy link

edelsohn commented Sep 14, 2023

ARM has released a patch and backported to GCC Releases 7-13. The backported patch has been applied to all actively maintained release branches of GCC. I don't know if ARM has asked the GCC Release Managers for an emergency release of the actively maintained branches, but the patch will be included in the next official instance of each release of GCC (GCC 11.5, GCC 12.4, GCC 13.3, and GCC 14.1).

Also, the Linux distros can backport the patch to their system compiler and optional compilers, e.g., Red Hat Developer Toolset. Just because the upstream release doesn't contain the patch, doesn't mean that packaged versions of GCC are lacking the patch. Most developers do not build GCC from sources, they use the package in their Linux distribution or an SDK provided by their vendor.

Yes, this is a security risk in code generated by GCC, but this is a best practices document, not an enumeration of all CVEs in all Open Source tools and options recommended in the best practices guide. This bug will not be relevant as inhibiting best practices for all times. As @ware states, this seems arbitrary.

@edelsohn
Copy link

Best practices is to use the -fstack-protector option. Yes, that option or any other recommended option in the best practices guide may contain a bug at any arbitrary point in the past or future lifecycle of the software mentioned. Is the best practices guide going to enumerate all previous bugs? Is it going to be updated each time that a bug or CVE is reported?

Maybe include a disclaimer that any of the recommended options can contain bugs and it's recommended that developers / users check for known vulnerabilities in the options and update to the latest release addressing bugs in the particular options that affect their platform.

@edelsohn
Copy link

Soon the Best Practices guide should recommend the newly proposed -fhardened option:
patch and thread

@thomasnyman
Copy link
Contributor Author

Thanks for the active input on this. I think the discussion here and in PR #235 is valuable in understanding the broader question of how to deal with issues which might undermine the effectiveness of the recommended options.

I think the point made by @edelsohn that trying to enumerate bugs is not something a best practices document should do is fair. I think not only would it be impractical to maintain, but I'd fear the guide would become too unwieldy to read as well.

At the same time, I'm troubled by leaving this particular issue unaddressed for the following reasons:

  1. the list affected versions of GCC is extensive
  2. in an ideal world the fix would propagate with upstream releases and distribution updates in a timely manner, but as @david-a-wheeler points out, especially in embedded systems development it is (unfortunately) common that updates to the compiler may not be done routinely but for specific, perhaps sometimes pressing reasons

For adopters of -fstack-protector CVE-2023-40339 may be such a pressing reason and there may be at least some value in getting the word out through outlets such as the Compiler Hardening Option Guide.

I however, do acknowledge that the wording in 2ff5054 strays too far in the 'enumerating CVEs' direction and do think it can be positioned more appropriately.

Based on the discussion so far, I see two possible ways forward:

  • a general disclaimer, as suggested by @edelsohn
  • for the broader question, articulating what criteria the guide should have for covering issues which may undermine the the effectiveness of the recommended options to avoid falling in the trap of covering arbitrary bugs
  • based on the above, either rewording Add note of CVE-2023-4039 to -fstack-protector description #235 or settling with the general disclaimer if CVE-2023-40339 falls outside the chosen criteria

The -fhardened options should be addressed in a separate issue, I've opened #240 for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants