[OE-core] [PATCH] meta:recipes-extended: stat fix security gaps
Khem Raj
raj.khem at gmail.com
Tue May 17 01:02:27 UTC 2016
> On May 16, 2016, at 5:21 PM, Plauchu Edwin <edwin.plauchu.camacho at 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 at linux.intel.com wrote:
>>>>>
>>>>> From: Edwin Plauchu <edwin.plauchu.camacho at 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 at 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 at 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 at lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 204 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.openembedded.org/pipermail/openembedded-core/attachments/20160516/4cb2baf9/attachment-0002.sig>
More information about the Openembedded-core
mailing list