g++ __VA_ARGS__ error

[Jan Synacek]

Hello,

I'm trying to compile the latest version of Warzone2100 on rawhide,
but I'm getting this error:

g++ -DHAVE_CONFIG_H -I. -I..  -DYY_NO_INPUT -D_REENTRANT
-I/usr/include/SDL2  -I/usr/include/libpng16   -I/usr/include/AL
-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DNDEBUG
-DWZ_DATADIR="\"/usr/share/warzone2100\""
-DLOCALEDIR="\"/usr/share/locale\"" -I.. -I../3rdparty
-I../3rdparty/quesoglc -I/usr/include/libdrm    -g -Wno-enum-compare
-Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wcast-align
-Wwrite-strings -Wpointer-arith -Wno-format-security
-I/usr/include/qt5/QtWidgets -I/usr/include/qt5
-I/usr/include/qt5/QtGui -I/usr/include/qt5
-I/usr/include/qt5/QtScript -I/usr/include/qt5
-I/usr/include/qt5/QtCore -I/usr/include/qt5  -O2 -g -pipe -Wall
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector-strong --param=ssp-buffer-size=4
-grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
-m64 -mtune=generic -fPIC -std=c++11 -fstack-protector -c -o
geometry.o geometry.cpp
In file included from ../lib/framework/frame.h:44:0,
                 from ../lib/framework/wzapp.h:24,
                 from frontend.cpp:27:
frontend.cpp: In function 'void startCampaignSelector()':
../lib/framework/string_ext.h:178:74: error: format not a string
literal and no format arguments [-Werror=format-security]
 #define ssprintf(dest, ...) snprintf((dest), sizeof(dest), __VA_ARGS__)

Could someone who understands g++ please advise how to fix this? I
don't quite understand why it doesn't work.

Cheers,
-- 
Jan Synacek
Software Engineer, Red Hat

[Tom Hughes]

On 11/07/16 13:09, Jan Synacek wrote:

>  In file included from ../lib/framework/frame.h:44:0,
>                   from ../lib/framework/wzapp.h:24,
>                   from frontend.cpp:27:
>  frontend.cpp: In function 'void startCampaignSelector()':
>  ../lib/framework/string_ext.h:178:74: error: format not a string
>  literal and no format arguments [-Werror=format-security]
>   #define ssprintf(dest, ...) snprintf((dest), sizeof(dest), __VA_ARGS__)
>
>  Could someone who understands g++ please advise how to fix this? I
>  don't quite understand why it doesn't work. 
Nothing to do with __VA_ARGS__ at all.

A non-constant string is being passed to ssprintf as the format, which 
is a potential security risk depending on the source of the string, and 
the default Fedora compile flags therefore prohibit it.

Tom

-- 
Tom Hughes (tom(a)compton.nu)
http://compton.nu/

[Jonathan Wakely]

On 11/07/16 14:09 +0200, Jan Synacek wrote:
> Hello,
>
> I'm trying to compile the latest version of Warzone2100 on rawhide,
> but I'm getting this error:
>
> g++ -DHAVE_CONFIG_H -I. -I..  -DYY_NO_INPUT -D_REENTRANT
> -I/usr/include/SDL2  -I/usr/include/libpng16   -I/usr/include/AL
> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DNDEBUG
> -DWZ_DATADIR="\"/usr/share/warzone2100\""
> -DLOCALEDIR="\"/usr/share/locale\"" -I.. -I../3rdparty
> -I../3rdparty/quesoglc -I/usr/include/libdrm    -g -Wno-enum-compare
> -Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wcast-align
> -Wwrite-strings -Wpointer-arith -Wno-format-security
> -I/usr/include/qt5/QtWidgets -I/usr/include/qt5
> -I/usr/include/qt5/QtGui -I/usr/include/qt5
> -I/usr/include/qt5/QtScript -I/usr/include/qt5
> -I/usr/include/qt5/QtCore -I/usr/include/qt5  -O2 -g -pipe -Wall
> -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
> -fstack-protector-strong --param=ssp-buffer-size=4
> -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
> -m64 -mtune=generic -fPIC -std=c++11 -fstack-protector -c -o
> geometry.o geometry.cpp
> In file included from ../lib/framework/frame.h:44:0,
>                  from ../lib/framework/wzapp.h:24,
>                  from frontend.cpp:27:
> frontend.cpp: In function 'void startCampaignSelector()':
> ../lib/framework/string_ext.h:178:74: error: format not a string
> literal and no format arguments [-Werror=format-security]
>  #define ssprintf(dest, ...) snprintf((dest), sizeof(dest), __VA_ARGS__)
>
> Could someone who understands g++ please advise how to fix this? I
> don't quite understand why it doesn't work. 
You need to look wherre the macro is being used, not the definition of
the macro.

It's telling you the format argument is not a string literal, and
there are no format args, suggesting it's being used something like:

  ssprintf(buf, str);

That's certainly questionable code, why use snprintf() if you don't
want any printf-style formatting? strncpy would do instead, or several
other approaches.

[Daniel P. Berrange]

On Mon, Jul 11, 2016 at 02:09:24PM +0200, Jan Synacek wrote:
>  Hello,
>  
>  I'm trying to compile the latest version of Warzone2100 on rawhide,
>  but I'm getting this error:
>  
>  g++ -DHAVE_CONFIG_H -I. -I..  -DYY_NO_INPUT -D_REENTRANT
>  -I/usr/include/SDL2  -I/usr/include/libpng16   -I/usr/include/AL
>  -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DNDEBUG
>  -DWZ_DATADIR="\"/usr/share/warzone2100\""
>  -DLOCALEDIR="\"/usr/share/locale\"" -I.. -I../3rdparty
>  -I../3rdparty/quesoglc -I/usr/include/libdrm    -g -Wno-enum-compare
>  -Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wcast-align
>  -Wwrite-strings -Wpointer-arith -Wno-format-security
>  -I/usr/include/qt5/QtWidgets -I/usr/include/qt5
>  -I/usr/include/qt5/QtGui -I/usr/include/qt5
>  -I/usr/include/qt5/QtScript -I/usr/include/qt5
>  -I/usr/include/qt5/QtCore -I/usr/include/qt5  -O2 -g -pipe -Wall
>  -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
>  -fstack-protector-strong --param=ssp-buffer-size=4
>  -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
>  -m64 -mtune=generic -fPIC -std=c++11 -fstack-protector -c -o
>  geometry.o geometry.cpp
>  In file included from ../lib/framework/frame.h:44:0,
>                   from ../lib/framework/wzapp.h:24,
>                   from frontend.cpp:27:
>  frontend.cpp: In function 'void startCampaignSelector()':
>  ../lib/framework/string_ext.h:178:74: error: format not a string
>  literal and no format arguments [-Werror=format-security]
>   #define ssprintf(dest, ...) snprintf((dest), sizeof(dest), __VA_ARGS__)
>  
>  Could someone who understands g++ please advise how to fix this? I
>  don't quite understand why it doesn't work. 
It means the code calling this ssprintf() macro is passing a variable,
instead of a literal string. This is potentially unsafe as the compiler
can't validate that the string data in this variable contains format
arguments that are compatible with the __VA_ARGS__ passed at the same
time.  This is quite commonly hit when people do not actually have any
variadic args at all, and just want to print out the string variable
as-is with no interpolation. The fix is usually to add a plain "%s"
format arg.

eg if you have a varaible  'char *somemsg' which contains the data
to print and you're calling ssprintf(somemsg), then you would want
to change it to ssprintf("%s", somemsg).  This avoids any danger if
'somemsg' could itself potentailly contain % format specifies


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

[Jonathan Wakely]

On 11/07/16 13:16 +0100, Daniel P. Berrange wrote:
> On Mon, Jul 11, 2016 at 02:09:24PM +0200, Jan Synacek wrote:
> > Hello,
> >
> > I'm trying to compile the latest version of Warzone2100 on rawhide,
> > but I'm getting this error:
> >
> > g++ -DHAVE_CONFIG_H -I. -I..  -DYY_NO_INPUT -D_REENTRANT
> > -I/usr/include/SDL2  -I/usr/include/libpng16   -I/usr/include/AL
> > -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DNDEBUG
> > -DWZ_DATADIR="\"/usr/share/warzone2100\""
> > -DLOCALEDIR="\"/usr/share/locale\"" -I.. -I../3rdparty
> > -I../3rdparty/quesoglc -I/usr/include/libdrm    -g -Wno-enum-compare
> > -Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wcast-align
> > -Wwrite-strings -Wpointer-arith -Wno-format-security
> > -I/usr/include/qt5/QtWidgets -I/usr/include/qt5
> > -I/usr/include/qt5/QtGui -I/usr/include/qt5
> > -I/usr/include/qt5/QtScript -I/usr/include/qt5
> > -I/usr/include/qt5/QtCore -I/usr/include/qt5  -O2 -g -pipe -Wall
> > -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
> > -fstack-protector-strong --param=ssp-buffer-size=4
> > -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
> > -m64 -mtune=generic -fPIC -std=c++11 -fstack-protector -c -o
> > geometry.o geometry.cpp
> > In file included from ../lib/framework/frame.h:44:0,
> >                  from ../lib/framework/wzapp.h:24,
> >                  from frontend.cpp:27:
> > frontend.cpp: In function 'void startCampaignSelector()':
> > ../lib/framework/string_ext.h:178:74: error: format not a string
> > literal and no format arguments [-Werror=format-security]
> >  #define ssprintf(dest, ...) snprintf((dest), sizeof(dest), __VA_ARGS__)
> >
> > Could someone who understands g++ please advise how to fix this? I
> > don't quite understand why it doesn't work.
>
> It means the code calling this ssprintf() macro is passing a variable,
> instead of a literal string. This is potentially unsafe as the compiler
> can't validate that the string data in this variable contains format
> arguments that are compatible with the __VA_ARGS__ passed at the same
> time.  This is quite commonly hit when people do not actually have any
> variadic args at all, and just want to print out the string variable
> as-is with no interpolation. The fix is usually to add a plain "%s"
> format arg.
>
> eg if you have a varaible  'char *somemsg' which contains the data
> to print and you're calling ssprintf(somemsg), then you would want
> to change it to ssprintf("%s", somemsg).  This avoids any danger if
> 'somemsg' could itself potentailly contain % format specifies 
Looks like it's coming from here:
https://github.com/Warzone2100/warzone2100/blob/master/src/frontend.cpp#L381

So the fix would be:

  ssprintf(hackList[i], "%s", list[i].name.toUtf8().constData());

[Jonathan Wakely]

On 11/07/16 13:22 +0100, Jonathan Wakely wrote:
> On 11/07/16 13:16 +0100, Daniel P. Berrange wrote:
> >On Mon, Jul 11, 2016 at 02:09:24PM +0200, Jan Synacek wrote:
> >>Hello,
> >>
> >>I'm trying to compile the latest version of Warzone2100 on rawhide,
> >>but I'm getting this error:
> >>
> >>g++ -DHAVE_CONFIG_H -I. -I..  -DYY_NO_INPUT -D_REENTRANT
> >>-I/usr/include/SDL2  -I/usr/include/libpng16   -I/usr/include/AL
> >>-D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DNDEBUG
> >>-DWZ_DATADIR="\"/usr/share/warzone2100\""
> >>-DLOCALEDIR="\"/usr/share/locale\"" -I.. -I../3rdparty
> >>-I../3rdparty/quesoglc -I/usr/include/libdrm    -g -Wno-enum-compare
> >>-Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wcast-align
> >>-Wwrite-strings -Wpointer-arith -Wno-format-security
> >>-I/usr/include/qt5/QtWidgets -I/usr/include/qt5
> >>-I/usr/include/qt5/QtGui -I/usr/include/qt5
> >>-I/usr/include/qt5/QtScript -I/usr/include/qt5
> >>-I/usr/include/qt5/QtCore -I/usr/include/qt5  -O2 -g -pipe -Wall
> >>-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
> >>-fstack-protector-strong --param=ssp-buffer-size=4
> >>-grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
> >>-m64 -mtune=generic -fPIC -std=c++11 -fstack-protector -c -o
> >>geometry.o geometry.cpp
> >>In file included from ../lib/framework/frame.h:44:0,
> >>                 from ../lib/framework/wzapp.h:24,
> >>                 from frontend.cpp:27:
> >>frontend.cpp: In function 'void startCampaignSelector()':
> >>../lib/framework/string_ext.h:178:74: error: format not a string
> >>literal and no format arguments [-Werror=format-security]
> >> #define ssprintf(dest, ...) snprintf((dest), sizeof(dest), __VA_ARGS__)
> >>
> >>Could someone who understands g++ please advise how to fix this? I
> >>don't quite understand why it doesn't work.
> >
> >It means the code calling this ssprintf() macro is passing a variable,
> >instead of a literal string. This is potentially unsafe as the compiler
> >can't validate that the string data in this variable contains format
> >arguments that are compatible with the __VA_ARGS__ passed at the same
> >time.  This is quite commonly hit when people do not actually have any
> >variadic args at all, and just want to print out the string variable
> >as-is with no interpolation. The fix is usually to add a plain "%s"
> >format arg.
> >
> >eg if you have a varaible  'char *somemsg' which contains the data
> >to print and you're calling ssprintf(somemsg), then you would want
> >to change it to ssprintf("%s", somemsg).  This avoids any danger if
> >'somemsg' could itself potentailly contain % format specifies
>
> Looks like it's coming from here:
> https://github.com/Warzone2100/warzone2100/blob/master/src/frontend.cpp#L381
>
> So the fix would be:
>
>  ssprintf(hackList[i], "%s", list[i].name.toUtf8().constData()); 
Although since this is C++ code, I have to wonder why they're using
macros and relying on sizeof(dest) which would do the wrong thing if
called with a pointer rather than an array. This would be safer:

template<std::size_t N, typename... Args>
  inline int
  ssprintf(char (&dest)[N], const char* fmt, Args... args)
  {
    return snprintf(dest, N, fmt, args...);
  }

[Jonathan Wakely]

On 11/07/16 13:27 +0100, Jonathan Wakely wrote:
> On 11/07/16 13:22 +0100, Jonathan Wakely wrote:
> >Looks like it's coming from here:
> >https://github.com/Warzone2100/warzone2100/blob/master/src/frontend.cpp#L381 
No check that list.size() <= 10 to prevent overflowing the hacklist
buffer ... eurgh, I hate looking at the code we package into Fedora.

[Jan Synacek]

On Mon, Jul 11, 2016 at 2:22 PM, Jonathan Wakely
<jwakely(a)fedoraproject.org&gt; wrote:
>  On 11/07/16 13:16 +0100, Daniel P. Berrange wrote:
> >
> > On Mon, Jul 11, 2016 at 02:09:24PM +0200, Jan Synacek wrote:
> >>
> >> Hello,
> >>
> >> I'm trying to compile the latest version of Warzone2100 on rawhide,
> >> but I'm getting this error:
> >>
> >> g++ -DHAVE_CONFIG_H -I. -I..  -DYY_NO_INPUT -D_REENTRANT
> >> -I/usr/include/SDL2  -I/usr/include/libpng16   -I/usr/include/AL
> >> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DNDEBUG
> >> -DWZ_DATADIR="\"/usr/share/warzone2100\""
> >> -DLOCALEDIR="\"/usr/share/locale\"" -I.. -I../3rdparty
> >> -I../3rdparty/quesoglc -I/usr/include/libdrm    -g -Wno-enum-compare
> >> -Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wcast-align
> >> -Wwrite-strings -Wpointer-arith -Wno-format-security
> >> -I/usr/include/qt5/QtWidgets -I/usr/include/qt5
> >> -I/usr/include/qt5/QtGui -I/usr/include/qt5
> >> -I/usr/include/qt5/QtScript -I/usr/include/qt5
> >> -I/usr/include/qt5/QtCore -I/usr/include/qt5  -O2 -g -pipe -Wall
> >> -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
> >> -fstack-protector-strong --param=ssp-buffer-size=4
> >> -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
> >> -m64 -mtune=generic -fPIC -std=c++11 -fstack-protector -c -o
> >> geometry.o geometry.cpp
> >> In file included from ../lib/framework/frame.h:44:0,
> >>                  from ../lib/framework/wzapp.h:24,
> >>                  from frontend.cpp:27:
> >> frontend.cpp: In function 'void startCampaignSelector()':
> >> ../lib/framework/string_ext.h:178:74: error: format not a string
> >> literal and no format arguments [-Werror=format-security]
> >>  #define ssprintf(dest, ...) snprintf((dest), sizeof(dest), __VA_ARGS__)
> >>
> >> Could someone who understands g++ please advise how to fix this? I
> >> don't quite understand why it doesn't work.
> >
> >
> > It means the code calling this ssprintf() macro is passing a variable,
> > instead of a literal string. This is potentially unsafe as the compiler
> > can't validate that the string data in this variable contains format
> > arguments that are compatible with the __VA_ARGS__ passed at the same
> > time.  This is quite commonly hit when people do not actually have any
> > variadic args at all, and just want to print out the string variable
> > as-is with no interpolation. The fix is usually to add a plain "%s"
> > format arg.
> >
> > eg if you have a varaible  'char *somemsg' which contains the data
> > to print and you're calling ssprintf(somemsg), then you would want
> > to change it to ssprintf("%s", somemsg).  This avoids any danger if
> > 'somemsg' could itself potentailly contain % format specifies
>
>
>  Looks like it's coming from here:
>  https://github.com/Warzone2100/warzone2100/blob/master/src/frontend.cpp#L381
>
>  So the fix would be:
>
>   ssprintf(hackList[i], "%s", list[i].name.toUtf8().constData()); 
Yep, that''s it, I didn't realize that.

My thanks to everyone!
-- 
Jan Synacek
Software Engineer, Red Hat

[David Howells]

Jan Synacek <jsynacek(a)redhat.com&gt; wrote:

>   #define ssprintf(dest, ...) snprintf((dest), sizeof(dest),
> __VA_ARGS__) 
Did you want:

#define ssprintf(dest, ...) snprintf((dest), sizeof(dest), ## __VA_ARGS__)

David