gisle / tcl.pm

Tcl extension module for Perl
http://search.cpan.org/dist/Tcl
9 stars 8 forks source link

Tcl.xs: use "%s" as format string to prevent warning #12

Closed chrstphrchvz closed 6 years ago

chrstphrchvz commented 6 years ago

Currently, warnings like the following might be printed for Tcl.xs:

Tcl.xs:1059:12: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
            croak(Tcl_GetStringResult(interp));
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
Tcl.xs:1059:12: note: treat the string as an argument to avoid this
            croak(Tcl_GetStringResult(interp));
                  ^
                  "%s", 

I added the "%s" format string to any croak(Tcl_GetStringResult(interp)) statements (6 total) as suggested to prevent this warning.

I'm not aware of any specific "risky" use cases, but I imagine that fixing would prevent legitimate Tcl output from being mistaken for a format string and resulting in "garbage" output.

chrstphrchvz commented 6 years ago

Thanks for the quick merge, however I hadn't yet checked RT for anything similar. It turns out these changes were part of a patch @jquelin submitted a few years ago: https://rt.cpan.org/Ticket/Display.html?id=78308. His patch includes some additional changes to Perl code Tcl.xs that might want to be added in , whereas I only updated XS code.

chrstphrchvz commented 6 years ago

Correction: his patch was also only for Tcl.xs and not Perl code, just that it had other changes besides using croak("%s", …).

There were two other changes in his patch:

  1. use warn("%s", …), which has since been applied as part of 85681a436a907b85954f416377aaff829276dc96
  2. cast occurrences of count to int when used in croak(); this hasn't been applied already. Nor did my compiler (clang) warn me about this.