Re: [PATCH] meta:recipes-extended: stat fix security gaps

[Plauchu Edwin <edwin.plauchu.camacho@linux.intel.com>]

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 <edwin.plauchu.camacho@linux.intel.com> 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 <edwin.plauchu.camacho@intel.com>
>>>>>>
>>>>>> 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 <edwin.plauchu.camacho@intel.com>
>>>>>> ---
>>>>>> 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 <edwin.plauchu.camacho@intel.com>
>>>>>> +
>>>>>> +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