From: Daniel Stenberg Date: Fri, 27 Dec 2024 08:21:56 +0000 (+0100) Subject: checksrc: introduce 'banfunc' to ban specific functions X-Git-Tag: curl-8_12_0~249 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c445b742;p=thirdparty%2Fcurl.git checksrc: introduce 'banfunc' to ban specific functions Use 'banfunc' and 'allowfunc' in .checksrc to specify which functions to ban or allow to be used. This saves us from having to edit the script going forward when we want to ban or allow specific functions. This replaces a set of previous rules and all banned functions are now checked with the BANNEDFUNC rule. There is a set of default banned functions, shown by invoking ./checksrc. Also, -a and -b options are added to specify allowed or banned functions on the command line. Closes #15835 --- diff --git a/docs/examples/.checksrc b/docs/examples/.checksrc index c7a5337394..dea90aaa1d 100644 --- a/docs/examples/.checksrc +++ b/docs/examples/.checksrc @@ -1,4 +1,3 @@ disable TYPEDEFSTRUCT disable SNPRINTF disable BANNEDFUNC -disable SSCANF diff --git a/lib/.checksrc b/lib/.checksrc index 9066946c89..3985707fd9 100644 --- a/lib/.checksrc +++ b/lib/.checksrc @@ -1,2 +1,5 @@ -enable STRERROR -enable STRNCPY +banfunc strerror +banfunc strncpy +banfunc sscanf +banfunc snprintf +banfunc vsnprint diff --git a/lib/strerror.c b/lib/strerror.c index 6b67a90588..32449c9fa1 100644 --- a/lib/strerror.c +++ b/lib/strerror.c @@ -891,7 +891,7 @@ const char *Curl_strerror(int err, char *buf, size_t buflen) } #else { - /* !checksrc! disable STRERROR 1 */ + /* !checksrc! disable BANNEDFUNC 1 */ const char *msg = strerror(err); if(msg) msnprintf(buf, buflen, "%s", msg); diff --git a/scripts/checksrc.pl b/scripts/checksrc.pl index d17f953144..a58163e304 100755 --- a/scripts/checksrc.pl +++ b/scripts/checksrc.pl @@ -47,10 +47,34 @@ my %ignore_set; my %ignore_used; my @ignore_line; +my %banfunc = ( + "gmtime" => 1, + "localtime" => 1, + "gets" => 1, + "strtok" => 1, + "sprintf" => 1, + "vsprintf" => 1, + "strcat" => 1, + "strncat" => 1, + "_mbscat" => 1, + "_mbsncat" => 1, + "_tcscat" => 1, + "_tcsncat" => 1, + "_wcscat" => 1, + "_wcsncat" => 1, + "LoadLibrary" => 1, + "LoadLibraryA" => 1, + "LoadLibraryW" => 1, + "LoadLibraryEx" => 1, + "LoadLibraryExA" => 1, + "LoadLibraryExW" => 1, + "_waccess" => 1, + "_access" => 1, + "access" => 1, + ); + my %warnings_extended = ( 'COPYRIGHTYEAR' => 'copyright year incorrect', - 'STRERROR', => 'strerror() detected', - 'STRNCPY', => 'strncpy() detected', 'STDERR', => 'stderr detected', ); @@ -92,14 +116,12 @@ my %warnings = ( 'RETURNNOSPACE' => 'return without space', 'SEMINOSPACE' => 'semicolon without following space', 'SIZEOFNOPAREN' => 'use of sizeof without parentheses', - 'SNPRINTF' => 'use of snprintf', 'SPACEAFTERPAREN' => 'space after open parenthesis', 'SPACEBEFORECLOSE' => 'space before a close parenthesis', 'SPACEBEFORECOMMA' => 'space before a comma', 'SPACEBEFOREPAREN' => 'space before an open parenthesis', 'SPACESEMICOLON' => 'space before semicolon', 'SPACESWITCHCOLON' => 'space before colon of switch label', - "SSCANF" => 'use of sscanf', 'TABS' => 'TAB characters not allowed', 'TRAILINGSPACE' => 'Trailing whitespace on the line', 'TYPEDEFSTRUCT' => 'typedefed struct', @@ -144,14 +166,14 @@ sub readlocalfile { if (/^\s*(#.*)/) { next; } - elsif (/^\s*enable ([A-Z]+)$/) { + elsif (/^enable ([A-Z]+)$/) { if(!defined($warnings_extended{$1})) { print STDERR "invalid warning specified in .checksrc: \"$1\"\n"; next; } $warnings{$1} = $warnings_extended{$1}; } - elsif (/^\s*disable ([A-Z]+)$/) { + elsif (/^disable ([A-Z]+)$/) { if(!defined($warnings{$1})) { print STDERR "invalid warning specified in .checksrc: \"$1\"\n"; next; @@ -159,6 +181,12 @@ sub readlocalfile { # Accept-list push @alist, $1; } + elsif (/^banfunc ([^ ]*)/) { + $banfunc{$1} = $1; + } + elsif (/^allowfunc ([^ ]*)/) { + undef $banfunc{$1}; + } else { die "Invalid format in $dir/.checksrc on line $i\n"; } @@ -223,27 +251,38 @@ $file = shift @ARGV; while(defined $file) { - if($file =~ /-D(.*)/) { + if($file =~ /^-D(.*)/) { $dir = $1; $file = shift @ARGV; next; } - elsif($file =~ /-W(.*)/) { + elsif($file =~ /^-W(.*)/) { $wlist .= " $1 "; $file = shift @ARGV; next; } - elsif($file =~ /-A(.+)/) { + elsif($file =~ /^-b(.*)/) { + $banfunc{$1} = $1; + print STDERR "ban use of \"$1\"\n"; + $file = shift @ARGV; + next; + } + elsif($file =~ /^-a(.*)/) { + undef $banfunc{$1}; + $file = shift @ARGV; + next; + } + elsif($file =~ /^-A(.+)/) { push @alist, $1; $file = shift @ARGV; next; } - elsif($file =~ /-i([1-9])/) { + elsif($file =~ /^-i([1-9])/) { $indent = $1 + 0; $file = shift @ARGV; next; } - elsif($file =~ /-m([0-9]+)/) { + elsif($file =~ /^-m([0-9]+)/) { $max_column = $1 + 0; $file = shift @ARGV; next; @@ -260,6 +299,8 @@ if(!$file) { print "checksrc.pl [option] [file2] ...\n"; print " Options:\n"; print " -A[rule] Accept this violation, can be used multiple times\n"; + print " -a[func] Allow use of this function\n"; + print " -b[func] Ban use of this function\n"; print " -D[DIR] Directory to prepend file names\n"; print " -h Show help output\n"; print " -W[file] Skip the given file - ignore all its flaws\n"; @@ -277,6 +318,11 @@ if(!$file) { } } print " [*] = disabled by default\n"; + + print "\nDetects and bans use of these functions:\n"; + for my $f (sort keys %banfunc) { + printf (" %-18s\n", $f); + } exit; } @@ -801,54 +847,23 @@ sub scanfile { } # scan for use of banned functions - if($l =~ /^(.*\W) - (gmtime|localtime| - gets| - strtok| - v?sprintf| - (str|_mbs|_tcs|_wcs)n?cat| - LoadLibrary(Ex)?(A|W)?| - _?w?access) - \s*\( - /x) { + my $bl = $l; + again: + if(($l =~ /^(.*?\W)(\w+)(\s*\()/x) && $banfunc{$2}) { + my $bad = $2; + my $prefix = $1; + my $suff = $3; checkwarn("BANNEDFUNC", - $line, length($1), $file, $ol, - "use of $2 is banned"); - } - # scan for use of sscanf. This is not a BANNEDFUNC to allow for - # individual enable/disable of this warning. - if($l =~ /^(.*\W)(sscanf)\s*\(/x) { - if($1 !~ /^ *\#/) { - # skip preprocessor lines - checkwarn("SSCANF", - $line, length($1), $file, $ol, - "use of $2 is banned"); - } - } - if($warnings{"STRERROR"}) { - # scan for use of banned strerror. This is not a BANNEDFUNC to - # allow for individual enable/disable of this warning. - if($l =~ /^(.*\W)(strerror)\s*\(/x) { - if($1 !~ /^ *\#/) { - # skip preprocessor lines - checkwarn("STRERROR", - $line, length($1), $file, $ol, - "use of $2 is banned"); - } - } - } - if($warnings{"STRNCPY"}) { - # scan for use of banned strncpy. This is not a BANNEDFUNC to - # allow for individual enable/disable of this warning. - if($l =~ /^(.*\W)(strncpy)\s*\(/x) { - if($1 !~ /^ *\#/) { - # skip preprocessor lines - checkwarn("STRNCPY", - $line, length($1), $file, $ol, - "use of $2 is banned"); - } - } - } + $line, length($prefix), $file, $ol, + "use of $bad is banned"); + my $replace = 'x' x (length($bad) + 1); + $prefix =~ s/\*/\\*/; + $suff =~ s/\(/\\(/; + $l =~ s/$prefix$bad$suff/$prefix$replace/; + goto again; + } + $l = $bl; # restore to pre-bannedfunc content + if($warnings{"STDERR"}) { # scan for use of banned stderr. This is not a BANNEDFUNC to # allow for individual enable/disable of this warning. @@ -861,12 +876,6 @@ sub scanfile { } } } - # scan for use of snprintf for curl-internals reasons - if($l =~ /^(.*\W)(v?snprintf)\s*\(/x) { - checkwarn("SNPRINTF", - $line, length($1), $file, $ol, - "use of $2 is banned"); - } # scan for use of non-binary fopen without the macro if($l =~ /^(.*\W)fopen\s*\([^,]*, *\"([^"]*)/) { diff --git a/src/.checksrc b/src/.checksrc index df9b1f0795..272515f78f 100644 --- a/src/.checksrc +++ b/src/.checksrc @@ -1,2 +1,3 @@ enable STDERR -enable STRNCPY +banfunc strncpy +banfunc sscanf diff --git a/tests/data/test1185 b/tests/data/test1185 index daea3101c8..6a4d43e291 100644 --- a/tests/data/test1185 +++ b/tests/data/test1185 @@ -16,7 +16,7 @@ checksrc -%SRCDIR/../scripts/checksrc.pl %LOGDIR/code%TESTNUMBER.c +%SRCDIR/../scripts/checksrc.pl -bmagicbad -balsobad %LOGDIR/code%TESTNUMBER.c /* test source code @@ -71,7 +71,7 @@ void startfunc(int a, int b) { } int a = sizeof int; - int a = snprintf(buffer, sizeof(buffer), "%d", 99); + int a = magicbad(buffer, alsobad(buffer), "%d", 99); int moo = hej?wrong:a>b; int moo2 = wrong2:(a)>(b); @@ -162,9 +162,12 @@ void startfunc(int a, int b) { ./%LOGDIR/code1185.c:52:16: warning: sizeof without parenthesis (SIZEOFNOPAREN) int a = sizeof int; ^ -./%LOGDIR/code1185.c:53:10: warning: use of snprintf is banned (SNPRINTF) - int a = snprintf(buffer, sizeof(buffer), "%d", 99); +./%LOGDIR/code1185.c:53:10: warning: use of magicbad is banned (BANNEDFUNC) + int a = magicbad(buffer, alsobad(buffer), "%d", 99); ^ +./%LOGDIR/code1185.c:53:27: warning: use of alsobad is banned (BANNEDFUNC) + int a = magicbad(buffer, alsobad(buffer), "%d", 99); + ^ ./%LOGDIR/code1185.c:54:21: warning: missing space before colon (NOSPACEC) int moo = hej?wrong:a>b; ^ @@ -201,7 +204,7 @@ void startfunc(int a, int b) { ./%LOGDIR/code1185.c:1:1: error: Missing closing comment (OPENCOMMENT) ^ -checksrc: 0 errors and 38 warnings +checksrc: 0 errors and 39 warnings 5 diff --git a/tests/libtest/.checksrc b/tests/libtest/.checksrc index 3d47f3e9a7..4483855d97 100644 --- a/tests/libtest/.checksrc +++ b/tests/libtest/.checksrc @@ -1,3 +1,3 @@ disable TYPEDEFSTRUCT -disable BANNEDFUNC -disable SSCANF +allowfunc localtime +allowfunc LoadLibrary diff --git a/tests/libtest/stub_gssapi.c b/tests/libtest/stub_gssapi.c index d581a91d5b..7fcbb641fa 100644 --- a/tests/libtest/stub_gssapi.c +++ b/tests/libtest/stub_gssapi.c @@ -30,8 +30,6 @@ #include "stub_gssapi.h" -/* !checksrc! disable SNPRINTF all */ - #define MAX_CREDS_LENGTH 250 #define APPROX_TOKEN_LEN 250 diff --git a/tests/server/.checksrc b/tests/server/.checksrc index 075b965819..ac086ea4c6 100644 --- a/tests/server/.checksrc +++ b/tests/server/.checksrc @@ -1,2 +1 @@ -enable STRNCPY -disable SSCANF +banfunc STRNCPY