9488 2016-04-19 14:57:14 +0000 security_flags.inc does not use format string vulnerability prevention flags 2016-05-06 09:36:32 +0000 1 1 7 Build System, Metadata & Runtime Security - Recipe Upgrade security unspecified x86 Multiple RESOLVED FIXED Medium normal 2.2 1 elena.reshetova joshuagloe alex.kanavin jku joshuagloe juro.bystricky richard.purdie ross.burton --- --- No (bug/feature does not impact docs) oldest_to_newest 61410 0 elena.reshetova 2016-04-19 14:57:14 +0000 security_flags.inc should also add the following flags: +# Add format string-specific flags +SECURITY_CFLAGS += "-Wformat -Wformat-security" +SECURITY_NO_PIE_CFLAGS += "-Wformat -Wformat-security" These flags warn developers about potential security vulnerabilities with regards to format string issues. 61418 1 alex.kanavin 2016-04-19 16:41:57 +0000 Why these flags in particular, and not -Wall -Wextra, or a broader subset of them? Also, these should be enabled only after agreeing who will fix the resulting errors and send the fixes upstream: it's not a small task. Enabling these warnings and then ignoring them is worse than not enabling them at all. 61503 2 richard.purdie 2016-04-21 14:49:43 +0000 These flags do check for quite specific security issues which is why I think they're being suggested. I do share the concerns about how much work is potentially involved in this :/. 61518 3 elena.reshetova 2016-04-21 16:23:21 +0000 I am trying to enable them now in Ostro scope: https://github.com/ostroproject/meta-ostro/pull/116/commits/a2779f89fc7f92b674053237326479352fd79c4f That touches also in my understanding oe core packages that are used in Ostro. So far I found only small amount of packages that need exceptions, so it might not be that bad as it seems. 61599 4 alex.kanavin 2016-04-25 09:50:28 +0000 So can you enable it for oe-core, issue 'bitbake world' and make a list of what starts to fail? It would be good to have some data, before making a decision of whether we proceed with this (against other things on our todo list). 61713 5 joshuagloe 2016-04-27 10:00:51 +0000 (In reply to comment #4) > So can you enable it for oe-core, issue 'bitbake world' and make a list of > what starts to fail? It would be good to have some data, before making a > decision of whether we proceed with this (against other things on our todo > list). I did a `bitbake world -k` in poky with DISTRO = "poky-lsb" (because that uses security_flags) and the following diff applied to my poky tree: diff --git a/meta/conf/distro/include/security_flags.inc b/meta/conf/distro/include/security_flags.inc index ff5f34e..1a50d6f 100644 --- a/meta/conf/distro/include/security_flags.inc +++ b/meta/conf/distro/include/security_flags.inc @@ -9,8 +9,8 @@ # -O0 which then results in a compiler warning. lcl_maybe_fortify = "${@base_conditional('DEBUG_BUILD','1','','-D_FORTIFY_SOURCE=2',d)}" -SECURITY_CFLAGS ?= "-fstack-protector-strong -pie -fpie ${lcl_maybe_fortify}" -SECURITY_NO_PIE_CFLAGS ?= "-fstack-protector-strong ${lcl_maybe_fortify}" +SECURITY_CFLAGS ?= "-fstack-protector-strong -pie -fpie ${lcl_maybe_fortify} -Wformat -Wformat-security" +SECURITY_NO_PIE_CFLAGS ?= "-fstack-protector-strong ${lcl_maybe_fortify} -Wformat -Wformat-security" SECURITY_LDFLAGS ?= "-fstack-protector-strong -Wl,-z,relro,-z,now" SECURITY_X_LDFLAGS ?= "-fstack-protector-strong -Wl,-z,relro" and nothing failed. I know, I too am a little shocked. 61714 6 alex.kanavin 2016-04-27 11:27:38 +0000 That's because it's only a warning, and will not make the build fail. You need to use -Werror= to turn it into an error. https://gcc.gnu.org/onlinedocs/gcc-4.3.1/gcc/Warning-Options.html Elena's Ostro patch has the same problem I believe, not sure how she found which packages need to be excepted from the check. 61765 7 joshuagloe 2016-04-28 08:28:59 +0000 Of course, thanks Alexander. I tested a world build with "-Wformat -Wformat-security -Werror=format-security" added to the SECURITY_CFLAGS. 13 recipes failed to build and I was able to complete a world build by removing the flags for the recipes in question, i.e.: SECURITY_CFLAGS_remove_pn-libuser = "-Wformat -Wformat-security -Werror=format-security" The failing recipes are: - busybox - console-tools - cmake - expect - gcc - gettext - kexec-tools - leafpad - libuser - ltp - makedevs - oh-puzzles - stat I haven't begun to look at how much work fixing those recipes is, or if we get that work for free in an upgrade. I did discover that these are amongst the default flags in Fedora[1][2] (since ) and Ubuntu[3] and part of the Debian hardening options[4], so there may be patches floating around. 1. https://fedoraproject.org/wiki/Format-Security-FAQ 2. https://fedorahosted.org/fesco/ticket/1185 3. https://wiki.ubuntu.com/ToolChain/CompilerFlags 4. https://wiki.debian.org/Hardening#DEB_BUILD_HARDENING_FORMAT_.28gcc.2Fg.2B-.2B-_-Wformat_-Wformat-security_-Werror.3Dformat-security.29 61770 8 jku 2016-04-28 10:31:01 +0000 (In reply to comment #7) Documenting what I mentioned in chat already: > - leafpad I'm currently in process of updating oe-core to GTK+3 (at least for things in core-image-sato). This almost certainly includes getting rid of leafpad. A possible replacement is l3afpad which is a fork and might still have the same problems. > - oh-puzzles oh-puzzles will be removed (we already have "puzzles" in oe-core and according to your test that doesn't have this problem). 61783 9 joshuagloe 2016-04-28 10:49:41 +0000 I've filed separate bugs (assigned to me) to resolve the string formatting issues in all of the identified recipes apart from Leafpad (where we should do the equivalent work on whatever replaces it, if required) and oh-puzzles (which will be removed). 61796 10 joshuagloe 2016-04-28 13:37:05 +0000 Patch submitted to OE-Core which elevates string-formatting issues to an error for all but 15 recipes. In addition to the 13 recipes mentioned earlier I also found zip and unzip fail to compile with -Werror=format-security http://lists.openembedded.org/pipermail/openembedded-core/2016-April/120753.html 61836 11 elena.reshetova 2016-05-02 07:43:09 +0000 (In reply to comment #6) > That's because it's only a warning, and will not make the build fail. > You need to use -Werror= to turn it into an error. > https://gcc.gnu.org/onlinedocs/gcc-4.3.1/gcc/Warning-Options.html > > Elena's Ostro patch has the same problem I believe, not sure how she found > which packages need to be excepted from the check. Some packages turn Werror by default, this is how I found it. Having it as a warning it is a fist step. 61837 12 elena.reshetova 2016-05-02 07:45:15 +0000 (In reply to comment #9) > I've filed separate bugs (assigned to me) to resolve the string formatting > issues in all of the identified recipes apart from Leafpad (where we should > do the equivalent work on whatever replaces it, if required) and oh-puzzles > (which will be removed). Thank you very much Joshua! 61840 13 alex.kanavin 2016-05-02 09:54:25 +0000 (In reply to comment #11) > Some packages turn Werror by default, this is how I found it. Having it as a > warning it is a fist step. Adding more compilation warnings is not useful in the context of openembedded build system. There are already hundreds of such warnings written into compile logs when packages are built, and nobody is going look at those logs until something fails. 61878 14 elena.reshetova 2016-05-03 05:48:29 +0000 (In reply to comment #13) > (In reply to comment #11) > > Some packages turn Werror by default, this is how I found it. Having it as a > > warning it is a fist step. > > Adding more compilation warnings is not useful in the context of > openembedded build system. There are already hundreds of such warnings > written into compile logs when packages are built, and nobody is going look > at those logs until something fails. Sure, but now it is enabled with Werror, so I don't think we have an issue here. When the patch done by Joshua is merged, we can close this bug and track only separate components bugs. Joshua, could you please add me as a watcher to separate bugs? 61880 15 joshuagloe 2016-05-03 08:50:38 +0000 (In reply to comment #14) > Joshua, could you please add me as a watcher to separate bugs? Done. 62012 16 joshuagloe 2016-05-06 09:36:32 +0000 Patch to add format string vulnerability prevention flags to SECURITY_CFLAGS has been merged: http://git.openembedded.org/openembedded-core/commit/?id=f335f8e744fb312b3eb599c331d08a9a6e5a8ff8 http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=30f9a5072d94f6c383b1ef5d25448af39691dce0