Is allowed in certain cases to override default Fedora compiler flags?

[Sergio Belkin]

Hi,

I'm trying to build a package for resubmission. However it fails with
messages like that

In file included from ./include/UpTools/UpLog.h:77,
                 from UpLog.cc:46:
UpLog.cc: In function 'void upOpenLogFileInternal(const char*, int, const
char*, int, int (*)(char*))':
./include/UpTools/UpLog.inl:63:30: error: format not a string literal and
no format arguments [-Werror=format-security]
   63 |    syslog(level, __VA_ARGS__ ); \
      |                              ^

End of message

The line in the code is:

n += snprintf(logbuf+n,MAXLOGLINE-1-n, __VA_ARGS__ ); \

And also this warning:

UpLog.cc: In function 'void upOpenLogInternal(const char*, int, int, int)':
./include/UpTools/UpLog.inl:69:11: warning: ignoring return value of
'ssize_t write(int, const void*, size_t)' declared with attribute
'warn_unused_result' [-Wunused-result]
   69 |    ::write(upLogFd,logbuf,n); \
      |    ~~~~~~~^~~~~~~~~~~~~~~~~~
./include/UpTools/UpLog.h:115:26: note: in expansion of macro '_UPLOG_'
  115 | #define UPLOG(level,...) _UPLOG_(level, __VA_ARGS__ )

End of message

The line in the code is :

 if(upLogPerror) ::write(2,logbuf,n); \

Regarding to " format not a string literal and no format arguments
[-Werror=format-security]" message.
Afaik instructions of kind printf(format,var1,var2,...) always be fail,
since it can't verify in compile time  that the format includes the number
of variables that appears later.

If the developer does not use entered formats by the user, the exploit
disappear, doesn't it?

So the question is: in this case I can override the Fedora compiler flags?

Thanks in advance!
-- 
--
Sergio Belkin
LPIC-2 Certified - http://www.lpi.org

[Jerry James]

On Wed, Jul 1, 2020 at 2:49 PM Sergio Belkin <sebelk(a)gmail.com&gt; wrote:
>  So the question is: in this case I can override the Fedora compiler
> flags? 
There may be a solution that doesn't require overriding the Fedora
compiler flags, but it is hard to tell without seeing more of the
code.  Where is the source code in question?
-- 
Jerry James
http://www.jamezone.org/

[Björn Persson]

Sergio Belkin wrote:
>  Regarding to " format not a string literal and no format
> arguments
>  [-Werror=format-security]" message.
>  Afaik instructions of kind printf(format,var1,var2,...) always be fail,
>  since it can't verify in compile time  that the format includes the number
>  of variables that appears later. 
GCC does exactly that. It has special knowledge of the printf family of
functions and verifies that the arguments match the format.

If you define a function that takes printf-like parameters, then you
should include an attribute like this:

void log(foo f, const char *format, ...) __attribute__((format(printf, 2, 3)));

Then GCC will verify that the arguments match the format in calls to
your function too.

>  If the developer does not use entered formats by the user, the
> exploit
>  disappear, doesn't it? 
Is it guaranteed that the string can never under any circumstances ever
possibly contain a percent sign? If so, it's probably safe – in the
current version of the program, but who knows what changes might be
made in the distant future?

Tell upstream to just add "%s" as the format string and be done with
it. If they find that burdensome, then that's because they made a bad
choice of programming language.

Björn Persson

[Steve Grubb]

On Wednesday, July 1, 2020 4:47:51 PM EDT Sergio Belkin wrote:
>  The line in the code is :
>  
>   if(upLogPerror) ::write(2,logbuf,n); \
>  
>  Regarding to " format not a string literal and no format arguments
>  [-Werror=format-security]" message.
>  Afaik instructions of kind printf(format,var1,var2,...) always be fail,
>  since it can't verify in compile time  that the format includes the number
>  of variables that appears later.
>  
>  If the developer does not use entered formats by the user, the exploit
>  disappear, doesn't it?
>  
>  So the question is: in this case I can override the Fedora compiler flags? 
This is pointing to a potential exploit in the code. In general, this is the 
pattern its detecting

char user_input[BUF_SIZE];

get_user_input(user_input);
printf(user_input);

The fix is to change the printf to

printf("%s", user_input);

Hope this helps...

-Steve

[Zbigniew Jędrzejewski-Szmek]

On Wed, Jul 01, 2020 at 05:47:51PM -0300, Sergio Belkin wrote:
>  So the question is: in this case I can override the Fedora compiler
> flags? 
Other people replied with suggestions how to make the code better, but
let me also answer this question directly:

yes you can, the guidelines say:

>   Adding to and overriding or filtering parts of these flags is
>   permitted if there’s a good reason to do so; the rationale for
>   doing so must be documented in the specfile. 
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_compiler_flags

[Björn Persson]

Zbigniew Jędrzejewski-Szmek wrote:
>  On Wed, Jul 01, 2020 at 05:47:51PM -0300, Sergio Belkin wrote:
>  > So the question is: in this case I can override the Fedora compiler flags?  
>  
>  Other people replied with suggestions how to make the code better, but
>  let me also answer this question directly:
>  
>  yes you can, the guidelines say:
>  
>  >  Adding to and overriding or filtering parts of these flags is
>  >  permitted if there’s a good reason to do so; the rationale for
>  >  doing so must be documented in the specfile.   
Note the words "good reason". As I understand it, Sergio's question is
whether this case is a good reason. In my opinion, incorrect use of
snprintf and write are bad reasons for overriding the compiler flags.
It's better to fix the actual problem than to silence the alarm.

Björn Persson

[Vitaly Zaitsev]

On 01.07.2020 22:47, Sergio Belkin wrote:
>  So the question is: in this case I can override the Fedora compiler
> flags? 
Don't do this, please. You should fix such potentially vulnerable parts
of code and send your patch to upstream.

-- 
Sincerely,
  Vitaly Zaitsev (vitaly(a)easycoding.org)

[Sergio Belkin]

El jue., 2 jul. 2020 a las 13:30, Vitaly Zaitsev via devel (<
devel(a)lists.fedoraproject.org>) escribió:

>  On 01.07.2020 22:47, Sergio Belkin wrote:
>  > So the question is: in this case I can override the Fedora compiler
>  flags?
>
>  Don't do this, please. You should fix such potentially vulnerable parts
>  of code and send your patch to upstream.
>
>  --
>  Sincerely,
>    Vitaly Zaitsev (vitaly(a)easycoding.org)
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct:
>  https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
>  https://lists.fedoraproject.org/archives/list/[e-mail]
>  
Thanks everyone, I guess the same thing goes for:

warning: ignoring return value of 'ssize_t write(int, const void*, size_t)'
declared with attribute 'warn_unused_result'

(The line in the source code  is if(upLogPerror) ::write(2,logbuf,n); \  )

doesn't it?




-- 
--
Sergio Belkin
LPIC-2 Certified - http://www.lpi.org

[Jonathan Wakely]

On 02/07/20 14:41 -0300, Sergio Belkin wrote:
> El jue., 2 jul. 2020 a las 13:30, Vitaly Zaitsev via devel (<
> devel(a)lists.fedoraproject.org>) escribió:
>
> > On 01.07.2020 22:47, Sergio Belkin wrote:
> > > So the question is: in this case I can override the Fedora compiler
> > flags?
> >
> > Don't do this, please. You should fix such potentially vulnerable parts
> > of code and send your patch to upstream.
> >
> > --
> > Sincerely,
> >   Vitaly Zaitsev (vitaly(a)easycoding.org)
> > _______________________________________________
> > devel mailing list -- devel(a)lists.fedoraproject.org
> > To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
> > Fedora Code of Conduct:
> > https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives:
> > https://lists.fedoraproject.org/archives/list/[e-mail]
> >
>
> Thanks everyone, I guess the same thing goes for:
>
> warning: ignoring return value of 'ssize_t write(int, const void*, size_t)'
> declared with attribute 'warn_unused_result'
>
> (The line in the source code  is if(upLogPerror) ::write(2,logbuf,n); \  )
>
> doesn't it? 
That's only a warning. Ideally the code should check the write, but
it's not actually preventing you building the package.

[Björn Persson]

Sergio Belkin wrote:
>  Thanks everyone, I guess the same thing goes for:
>  
>  warning: ignoring return value of 'ssize_t write(int, const void*, size_t)'
>  declared with attribute 'warn_unused_result'
>  
>  (The line in the source code  is if(upLogPerror) ::write(2,logbuf,n); \  )
>  
>  doesn't it? 
Ignoring the result of write is usually a serious bug. However, that
line looks like it writes a log message to the standard error stream,
and in that specific case there might not be anything meaningful the
program can do if write fails, if it doesn't want to terminate with an
error status. Logging an error message about how logging failed could
easily lead to infinite recursion.

So depending on what other error reporting mechanisms are available to
the program, it may be reasonable to ignore the function result in this
case, but it should be done by tweaking the code to silence the
compiler warning for that call only. Disabling -Wunused-result for the
whole program is not a good idea.

Björn Persson

[Kevin Kofler]

Sergio Belkin wrote:
>  So the question is: in this case I can override the Fedora compiler
> flags? 
Just append -Wno-error=format-security, and if you want to get this out of 
the build log entirely, also -Wno-format-security. You don't have to 
actually remove any of the Fedora flags, appended flags will override the 
earlier ones. :-)

        Kevin Kofler