From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mail.openembedded.org (Postfix) with ESMTP id D00807687B for ; Tue, 17 May 2016 02:15:44 +0000 (UTC) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 16 May 2016 19:15:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,323,1459839600"; d="scan'208";a="978358131" Received: from pianodaemon-contraption.zpn.intel.com (HELO [10.219.128.59]) ([10.219.128.59]) by orsmga002.jf.intel.com with ESMTP; 16 May 2016 19:15:44 -0700 To: Khem Raj References: <1463429996-24056-1-git-send-email-edwin.plauchu.camacho@linux.intel.com> <1407DCB5-D6A6-45D4-8F5B-DE43C8F74E59@gmail.com> <573A3DA4.9060900@linux.intel.com> <1463437315.2768.29.camel@intel.com> <573A641D.90808@linux.intel.com> <57F72925-B08E-45A1-A259-EDE409DC21C2@gmail.com> From: Plauchu Edwin Message-ID: <573A7EDD.4010002@linux.intel.com> Date: Mon, 16 May 2016 21:15:57 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <57F72925-B08E-45A1-A259-EDE409DC21C2@gmail.com> Cc: "openembedded-core@lists.openembedded.org" Subject: Re: [PATCH] meta:recipes-extended: stat fix security gaps X-BeenThere: openembedded-core@lists.openembedded.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Patches and discussions about the oe-core layer List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 May 2016 02:15:45 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit This is the version fputs call http://lists.openembedded.org/pipermail/openembedded-core/2016-May/121584.html On 16/05/16 20:02, Khem Raj wrote: >> On May 16, 2016, at 5:21 PM, Plauchu Edwin wrote: >> >> Ok Bill >> >> I rewrote the patch without using macros http://lists.openembedded.org/pipermail/openembedded-core/2016-May/121581.html >> > you did not address the other comment about using printf properly. > >> On 16/05/16 17:21, Randle, William C wrote: >>> On Mon, 2016-05-16 at 16:37 -0500, Plauchu Edwin wrote: >>>> On 16/05/16 16:28, Khem Raj wrote: >>>>>> On May 16, 2016, at 1:19 PM, edwin.plauchu.camacho@linux.intel.com wrote: >>>>>> >>>>>> From: Edwin Plauchu >>>>>> >>>>>> This patch avoids stat fails to compile with compiler flags which elevate >>>>>> common string formatting issues into an error (-Wformat -Wformat-security >>>>>> -Werror=format-security). >>>>>> >>>>>> [YOCTO #9550] >>>>>> >>>>>> Signed-off-by: Edwin Plauchu >>>>>> --- >>>>>> meta/conf/distro/include/security_flags.inc | 1 - >>>>>> .../stat/stat-3.3/fix-security-format.patch | 77 >>>>>> ++++++++++++++++++++++ >>>>>> meta/recipes-extended/stat/stat_3.3.bb | 1 + >>>>>> 3 files changed, 78 insertions(+), 1 deletion(-) >>>>>> create mode 100644 meta/recipes-extended/stat/stat-3.3/fix-security- >>>>>> format.patch >>>>>> >>>>>> diff --git a/meta/conf/distro/include/security_flags.inc >>>>>> b/meta/conf/distro/include/security_flags.inc >>>>>> index 7a91cec..5ae6dd8 100644 >>>>>> --- a/meta/conf/distro/include/security_flags.inc >>>>>> +++ b/meta/conf/distro/include/security_flags.inc >>>>>> @@ -105,7 +105,6 @@ SECURITY_STRINGFORMAT_pn-gettext = "" >>>>>> SECURITY_STRINGFORMAT_pn-kexec-tools = "" >>>>>> SECURITY_STRINGFORMAT_pn-makedevs = "" >>>>>> SECURITY_STRINGFORMAT_pn-oh-puzzles = "" >>>>>> -SECURITY_STRINGFORMAT_pn-stat = "" >>>>>> SECURITY_STRINGFORMAT_pn-unzip = "" >>>>>> SECURITY_STRINGFORMAT_pn-zip = "" >>>>>> >>>>>> diff --git a/meta/recipes-extended/stat/stat-3.3/fix-security-format.patch >>>>>> b/meta/recipes-extended/stat/stat-3.3/fix-security-format.patch >>>>>> new file mode 100644 >>>>>> index 0000000..7d9f8df >>>>>> --- /dev/null >>>>>> +++ b/meta/recipes-extended/stat/stat-3.3/fix-security-format.patch >>>>>> @@ -0,0 +1,77 @@ >>>>>> +meta: recipes-extended: Fixing security formatting issues on stat >>>>>> + >>>>>> +Fix security formatting issues related to printf without NULL argument >>>>>> + >>>>>> +stat.c: In function 'print_human_access': >>>>>> +stat.c:292:13: error: format not a string literal and no format arguments >>>>>> [-Werror=format-security] >>>>>> + printf (access); >>>>>> + ^ >>>>>> +stat.c: In function 'print_human_time': >>>>>> +stat.c:299:57: error: format not a string literal and no format arguments >>>>>> [-Werror=format-security] >>>>>> + if (strftime(str, 40, "%c", localtime(t)) > 0) printf(str); >>>>>> + ^ >>>>>> +stat.c: In function 'print_it': >>>>>> +stat.c:613:6: error: format not a string literal and no format arguments >>>>>> [-Werror=format-security] >>>>>> + printf(b); >>>>>> + ^ >>>>>> +stat.c:642:6: error: format not a string literal and no format arguments >>>>>> [-Werror=format-security] >>>>>> + printf(b); >>>>>> + ^ >>>>>> + >>>>>> +[YOCTO #9550] >>>>>> +[https://bugzilla.yoctoproject.org/show_bug.cgi?id=9550] >>>>>> + >>>>>> +Upstream-Status: Pending >>>>>> + >>>>>> +Signed-off-by: Edwin Plauchu >>>>>> + >>>>>> +diff --git a/stat.c b/stat.c >>>>>> +index 1ed07a9..351ab54 100644 >>>>>> +--- a/stat.c >>>>>> ++++ b/stat.c >>>>>> +@@ -21,6 +21,8 @@ >>>>>> + >>>>>> + #include "fs.h" >>>>>> + >>>>>> ++#define __PRINT(STR) printf (STR,NULL) >>>>>> ++ >>>>> Can we use proper formatting string here something like >>>>> printf(“%s”, access ); >>>>> >>>>> or use fputs() Call instead >>>> With fputs we need to specify stdout stream and >>>> the printf "%s" option needs a little bit more processing in formatting. >>>> >>>> The actual change covers the security considerations with minimal add of >>>> NULL if you >>>> know why the another ways will be better please tell me. >>>> >>>> Thanks in advance >>>> Edwin Plauchu >>>>>> + void print_human_type(unsigned short mode) >>>>>> + { >>>>>> + switch (mode & S_IFMT) >>>>>> +@@ -289,15 +291,15 @@ void print_human_access(struct stat *statbuf) >>>>>> + default: >>>>>> + access[0] = '?'; >>>>>> + } >>>>>> +- printf (access); >>>>>> ++ __PRINT(access); >>>>>> + } >>>>>> + >>>>>> + void print_human_time(time_t *t) >>>>>> + { >>>>>> + char str[40]; >>>>>> + >>>>>> +- if (strftime(str, 40, "%c", localtime(t)) > 0) printf(str); >>>>>> +- else printf("Cannot calculate human readable time, sorry"); >>>>>> ++ if (strftime(str, 40, "%c", localtime(t)) > 0) __PRINT(str); >>>>>> ++ else __PRINT("Cannot calculate human readable time, sorry"); >>>>>> + } >>>>>> + >>>>>> + /* print statfs info */ >>>>>> +@@ -610,7 +612,7 @@ void print_it(char *masterformat, char *filename, >>>>>> + { >>>>>> + strcpy (pformat, "%"); >>>>>> + *m++ = '\0'; >>>>>> +- printf(b); >>>>>> ++ __PRINT(b); >>>>>> + >>>>>> + /* copy all format specifiers to our format string */ >>>>>> + while (isdigit(*m) || strchr("#0-+. I", *m)) >>>>>> +@@ -639,7 +641,7 @@ void print_it(char *masterformat, char *filename, >>>>>> + } >>>>>> + else >>>>>> + { >>>>>> +- printf(b); >>>>>> ++ __PRINT(b); >>>>>> + b = NULL; >>>>>> + } >>>>>> + } >>> Is there a particular reason you used a macro for this package when all the >>> others you submitted so far use printf(arg, NULL) directly? I think it would be >>> good to be consistent. >>> >>> -Bill >>> >> -- >> _______________________________________________ >> Openembedded-core mailing list >> Openembedded-core@lists.openembedded.org >> http://lists.openembedded.org/mailman/listinfo/openembedded-core