From 69084a95bb5191b9a7638a4b6348ac7e848a069e Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Tue, 26 Feb 2019 13:09:04 +0100 Subject: [PATCH] Construction of string literals without using plus operators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The programming language “Python” supports string literal concatenation without the usage of additional plus operators as standard functionality. https://docs.python.org/3/reference/lexical_analysis.html#string-literal-concatenation Thus omit unnecessary operator specifications. Signed-off-by: Markus Elfring --- flawfinder | 129 ++++++++++++++++++++++++++--------------------------- 1 file changed, 64 insertions(+), 65 deletions(-) diff --git a/flawfinder b/flawfinder index 8bbf1af..5cbfdfb 100755 --- a/flawfinder +++ b/flawfinder @@ -184,9 +184,9 @@ diff_line_del = re.compile(r'^-[^-].*') # Also, "newfile" can have " (comment)" postpended. Find and eliminate this. # Note that the expression below is Y10K (and Y100K) ready. :-). diff_findjunk = re.compile( - r'^(?P.*)(' + - r'(\s\d\d\d\d+-\d\d-\d\d\s+\d\d:\d[0-9:.]+Z?(\s+[\-\+0-9A-Z]+)?)|' + - r'(\s[A-Za-z][a-z]+\s[A-za-z][a-z]+\s\d+\s\d+:\d[0-9:.]+Z?' + + r'^(?P.*)(' + r'(\s\d\d\d\d+-\d\d-\d\d\s+\d\d:\d[0-9:.]+Z?(\s+[\-\+0-9A-Z]+)?)|' + r'(\s[A-Za-z][a-z]+\s[A-za-z][a-z]+\s\d+\s\d+:\d[0-9:.]+Z?' r'(\s[\-\+0-9]*)?\s\d\d\d\d+)|' r'(\s\(.*\)))\s*$' ) @@ -273,7 +273,7 @@ def load_patch_info(input_patch_file): if hunk_match: if patched_filename == "": error( - "wrong type of patch file : " + + "wrong type of patch file : " "we have a line number without having seen a filename" ) sys.exit(13) @@ -577,8 +577,8 @@ def extract_c_parameters(text, pos=0): # In practice, this doesn't seem to be a problem; gettext() is usually # wrapped around the entire parameter. # The ?s makes it posible to match multi-line strings. -gettext_pattern = re.compile(r'(?s)^\s*' + 'gettext' + r'\s*\((.*)\)\s*$') -undersc_pattern = re.compile(r'(?s)^\s*' + '_(T(EXT)?)?' + r'\s*\((.*)\)\s*$') +gettext_pattern = re.compile(r'(?s)^\s*' 'gettext' r'\s*\((.*)\)\s*$') +undersc_pattern = re.compile(r'(?s)^\s*' '_(T(EXT)?)?' r'\s*\((.*)\)\s*$') def strip_i18n(text): @@ -642,7 +642,7 @@ def c_buffer(hit): add_warning(hit) -p_dangerous_strncat = re.compile(r'^\s*sizeof\s*(\(\s*)?[A-Za-z_$0-9]+' + +p_dangerous_strncat = re.compile(r'^\s*sizeof\s*(\(\s*)?[A-Za-z_$0-9]+' r'\s*(\)\s*)?(-\s*1\s*)?$') # This is a heuristic: constants in C are usually given in all # upper case letters. Yes, this need not be true, but it's true often @@ -680,7 +680,7 @@ def c_strncat(hit): hit.level = 5 hit.note = ( "Risk is high; the length parameter appears to be a constant, " - + "instead of computing the number of characters left.") + "instead of computing the number of characters left.") add_warning(hit) return c_buffer(hit) @@ -758,9 +758,9 @@ def c_scanf(hit): elif p_low_risk_scanf_format.search(source): # This is often okay, but sometimes extremely serious. hit.level = 1 - hit.warning = ("It's unclear if the %s limit in the " + + hit.warning = ("It's unclear if the %s limit in the " "format string is small enough (CWE-120)") - hit.suggestion = ("Check that the limit is sufficiently " + + hit.suggestion = ("Check that the limit is sufficiently " "small, or use a different input function") else: # No risky scanf request. @@ -770,16 +770,16 @@ def c_scanf(hit): hit.note = "No risky scanf format detected." else: # Format isn't a constant. - hit.note = ("If the scanf format is influenceable " + + hit.note = ("If the scanf format is influenceable " "by an attacker, it's exploitable.") add_warning(hit) -p_dangerous_multi_byte = re.compile(r'^\s*sizeof\s*(\(\s*)?[A-Za-z_$0-9]+' + +p_dangerous_multi_byte = re.compile(r'^\s*sizeof\s*(\(\s*)?[A-Za-z_$0-9]+' r'\s*(\)\s*)?(-\s*1\s*)?$') p_safe_multi_byte = re.compile( - r'^\s*sizeof\s*(\(\s*)?[A-Za-z_$0-9]+\s*(\)\s*)?' + - r'/\s*sizeof\s*\(\s*?[A-Za-z_$0-9]+\s*' + r'\[\s*0\s*\]\)\s*(-\s*1\s*)?$') + r'^\s*sizeof\s*(\(\s*)?[A-Za-z_$0-9]+\s*(\)\s*)?' + r'/\s*sizeof\s*\(\s*?[A-Za-z_$0-9]+\s*\[\s*0\s*\]\)\s*(-\s*1\s*)?$') def c_multi_byte_to_wide_char(hit): @@ -792,7 +792,7 @@ def c_multi_byte_to_wide_char(hit): hit.level = 5 hit.note = ( "Risk is high, it appears that the size is given as bytes, but the " - + "function requires size as characters.") + "function requires size as characters.") elif p_safe_multi_byte.search(num_chars_to_copy): # This isn't really risk-free, since it might not be the destination, # or the destination might be a character array (if it's a char pointer, @@ -897,7 +897,7 @@ c_ruleset = { 1, # Low risk level, because this is often used correctly when FIXING security # problems, and raising it to a higher risk level would cause many false # positives. - "Easily used incorrectly; doesn't always \\0-terminate or " + + "Easily used incorrectly; doesn't always \\0-terminate or " "check for invalid pointers [MS-banned] (CWE-120)", "", "buffer", "", {}), @@ -906,7 +906,7 @@ c_ruleset = { 1, # Low risk level, because this is often used correctly when FIXING security # problems, and raising it to a higher risk levle would cause many false # positives. - "Easily used incorrectly; doesn't always \\0-terminate or " + + "Easily used incorrectly; doesn't always \\0-terminate or " "check for invalid pointers [MS-banned] (CWE-120)", "", "buffer", "", {}), @@ -933,9 +933,9 @@ c_ruleset = { "buffer", "", {}), "char|TCHAR|wchar_t": # This isn't really a function call, but it works. (c_static_array, 2, - "Statically-sized arrays can be improperly restricted, " + + "Statically-sized arrays can be improperly restricted, " "leading to potential overflows or other issues (CWE-119!/CWE-120)", - "Perform bounds checking, use functions that limit length, " + + "Perform bounds checking, use functions that limit length, " "or ensure that the size is larger than the maximum possible length", "buffer", "", {'extract_lookahead': 1}), @@ -965,21 +965,21 @@ c_ruleset = { # The "syslog" hook will raise "format" issues. "syslog": (c_printf, 4, - "If syslog's format strings can be influenced by an attacker, " + + "If syslog's format strings can be influenced by an attacker, " "they can be exploited (CWE-134)", "Use a constant format string for syslog", "format", "", {'format_position': 2}), "snprintf|vsnprintf|_snprintf|_sntprintf|_vsntprintf": (c_printf, 4, - "If format strings can be influenced by an attacker, they can be " + + "If format strings can be influenced by an attacker, they can be " "exploited, and note that sprintf variations do not always \\0-terminate (CWE-134)", "Use a constant for the format specification", "format", "", {'format_position': 3}), "scanf|vscanf|wscanf|_tscanf|vwscanf": (c_scanf, 4, - "The scanf() family's %s operation, without a limit specification, " + + "The scanf() family's %s operation, without a limit specification, " "permits buffer overflows (CWE-120, CWE-20)", "Specify a limit to %s, or use a different input function", "buffer", "", {'input': 1}), @@ -996,8 +996,8 @@ c_ruleset = { # Often this isn't really a risk, and even when it is, at worst it # often causes a program crash (and nothing worse). 1, - "Does not handle strings that are not \\0-terminated; " + - "if given one it may perform an over-read (it could cause a crash " + + "Does not handle strings that are not \\0-terminated; " + "if given one it may perform an over-read (it could cause a crash " "if unprotected) (CWE-126)", "", "buffer", "", {}), @@ -1023,10 +1023,10 @@ c_ruleset = { "realpath": (normal, 3, - "This function does not protect against buffer overflows, " + + "This function does not protect against buffer overflows, " "and some implementations can overflow internally (CWE-120/CWE-785!)", - "Ensure that the destination buffer is at least of size MAXPATHLEN, and" + - "to protect against implementation problems, the input argument " + + "Ensure that the destination buffer is at least of size MAXPATHLEN, and" + "to protect against implementation problems, the input argument " "should also be checked to ensure it is no larger than MAXPATHLEN", "buffer", "dangers-c", {}), @@ -1052,43 +1052,43 @@ c_ruleset = { "access": # ???: TODO: analyze TOCTOU more carefully. (normal, 4, - "This usually indicates a security flaw. If an " + - "attacker can change anything along the path between the " + - "call to access() and the file's actual use (e.g., by moving " + + "This usually indicates a security flaw. If an " + "attacker can change anything along the path between the " + "call to access() and the file's actual use (e.g., by moving " "files), the attacker can exploit the race condition (CWE-362/CWE-367!)", - "Set up the correct permissions (e.g., using setuid()) and " + + "Set up the correct permissions (e.g., using setuid()) and " "try to open the file directly", "race", "avoid-race#atomic-filesystem", {}), "chown": (normal, 5, - "This accepts filename arguments; if an attacker " + + "This accepts filename arguments; if an attacker " "can move those files, a race condition results. (CWE-362)", "Use fchown( ) instead", "race", "", {}), "chgrp": (normal, 5, - "This accepts filename arguments; if an attacker " + + "This accepts filename arguments; if an attacker " "can move those files, a race condition results. (CWE-362)", "Use fchgrp( ) instead", "race", "", {}), "chmod": (normal, 5, - "This accepts filename arguments; if an attacker " + + "This accepts filename arguments; if an attacker " "can move those files, a race condition results. (CWE-362)", "Use fchmod( ) instead", "race", "", {}), "vfork": (normal, 2, - "On some old systems, vfork() permits race conditions, and it's " + + "On some old systems, vfork() permits race conditions, and it's " "very difficult to use correctly (CWE-362)", "Use fork() instead", "race", "", {}), "readlink": (normal, 5, - "This accepts filename arguments; if an attacker " + - "can move those files or change the link content, " + - "a race condition results. " + + "This accepts filename arguments; if an attacker " + "can move those files or change the link content, " + "a race condition results. " "Also, it does not terminate with ASCII NUL. (CWE-362, CWE-20)", # This is often just a bad idea, and it's hard to suggest a # simple alternative: @@ -1134,7 +1134,7 @@ c_ruleset = { # Windows. TODO: Detect correct usage approaches and ignore it. "GetTempFileName": (normal, 3, - "Temporary file race condition in certain cases " + + "Temporary file race condition in certain cases " "(e.g., if run as SYSTEM in many versions of Windows) (CWE-377)", "", "tmpfile", "avoid-race", {}), @@ -1143,7 +1143,7 @@ c_ruleset = { "execl|execlp|execle|execv|execvp|system|popen|WinExec|ShellExecute": (normal, 4, "This causes a new program to execute and is difficult to use safely (CWE-78)", - "try using a library call that implements the same functionality " + + "try using a library call that implements the same functionality " "if available", "shell", "", {}), @@ -1160,16 +1160,16 @@ c_ruleset = { "CreateProcess": (c_hit_if_null, 3, "This causes a new process to execute and is difficult to use safely (CWE-78)", - "Specify the application path in the first argument, NOT as part of the second, " + + "Specify the application path in the first argument, NOT as part of the second, " "or embedded spaces could allow an attacker to force a different program to run", "shell", "", {'check_for_null': 1}), "atoi|atol|_wtoi|_wtoi64": (normal, 2, - "Unless checked, the resulting number can exceed the expected range " + + "Unless checked, the resulting number can exceed the expected range " "(CWE-190)", - "If source untrusted, check both minimum and maximum, even if the" + - " input had no minus sign (large numbers can roll over into negative" + + "If source untrusted, check both minimum and maximum, even if the" + " input had no minus sign (large numbers can roll over into negative" " number; consider saving to an unsigned value if that is intended)", "integer", "dangers-c", {}), @@ -1182,11 +1182,11 @@ c_ruleset = { "crypt|crypt_r": (normal, 4, - "The crypt functions use a poor one-way hashing algorithm; " + - "since they only accept passwords of 8 characters or fewer " + - "and only a two-byte salt, they are excessively vulnerable to " + + "The crypt functions use a poor one-way hashing algorithm; " + "since they only accept passwords of 8 characters or fewer " + "and only a two-byte salt, they are excessively vulnerable to " "dictionary attacks given today's faster computing equipment (CWE-327)", - "Use a different algorithm, such as SHA-256, with a larger, " + + "Use a different algorithm, such as SHA-256, with a larger, " "non-repeating salt", "crypto", "", {}), @@ -1194,7 +1194,7 @@ c_ruleset = { "EVP_des_ecb|EVP_des_cbc|EVP_des_cfb|EVP_des_ofb|EVP_desx_cbc": (normal, 4, "DES only supports a 56-bit keysize, which is too small given today's computers (CWE-327)", - "Use a different patent-free encryption algorithm with a larger keysize, " + + "Use a different patent-free encryption algorithm with a larger keysize, " "such as 3DES or AES", "crypto", "", {}), @@ -1202,38 +1202,37 @@ c_ruleset = { "EVP_rc4_40|EVP_rc2_40_cbc|EVP_rc2_64_cbc": (normal, 4, "These keysizes are too small given today's computers (CWE-327)", - "Use a different patent-free encryption algorithm with a larger keysize, " + + "Use a different patent-free encryption algorithm with a larger keysize, " "such as 3DES or AES", "crypto", "", {}), "chroot": (normal, 3, "chroot can be very helpful, but is hard to use correctly (CWE-250, CWE-22)", - "Make sure the program immediately chdir(\"/\")," + - " closes file descriptors," + - " and drops root privileges, and that all necessary files" + + "Make sure the program immediately chdir(\"/\"), closes file descriptors," + " and drops root privileges, and that all necessary files" " (and no more!) are in the new root", "misc", "", {}), "getenv|curl_getenv": - (normal, 3, "Environment variables are untrustable input if they can be" + - " set by an attacker. They can have any content and" + + (normal, 3, "Environment variables are untrustable input if they can be" + " set by an attacker. They can have any content and" " length, and the same variable can be set more than once (CWE-807, CWE-20)", "Check environment variables carefully before using them", "buffer", "", {'input': 1}), "g_get_home_dir": - (normal, 3, "This function is synonymous with 'getenv(\"HOME\")';" + - "it returns untrustable input if the environment can be" + - "set by an attacker. It can have any content and length, " + + (normal, 3, "This function is synonymous with 'getenv(\"HOME\")';" + "it returns untrustable input if the environment can be" + "set by an attacker. It can have any content and length, " "and the same variable can be set more than once (CWE-807, CWE-20)", "Check environment variables carefully before using them", "buffer", "", {'input': 1}), "g_get_tmp_dir": - (normal, 3, "This function is synonymous with 'getenv(\"TMP\")';" + - "it returns untrustable input if the environment can be" + - "set by an attacker. It can have any content and length, " + + (normal, 3, "This function is synonymous with 'getenv(\"TMP\")';" + "it returns untrustable input if the environment can be" + "set by an attacker. It can have any content and length, " "and the same variable can be set more than once (CWE-807, CWE-20)", "Check environment variables carefully before using them", "buffer", "", {'input': 1}), @@ -1242,8 +1241,8 @@ c_ruleset = { # These are Windows-unique: # TODO: Should have lower risk if the program checks return value. - "RpcImpersonateClient|ImpersonateLoggedOnUser|CoImpersonateClient|" + - "ImpersonateNamedPipeClient|ImpersonateDdeClientWindow|ImpersonateSecurityContext|" + + "RpcImpersonateClient|ImpersonateLoggedOnUser|CoImpersonateClient|" + "ImpersonateNamedPipeClient|ImpersonateDdeClientWindow|ImpersonateSecurityContext|" "SetThreadToken": (normal, 4, "If this call fails, the program could fail to drop heightened privileges (CWE-250)", "Make sure the return value is checked, and do not continue if a failure is reported", @@ -1266,7 +1265,7 @@ c_ruleset = { "SetSecurityDescriptorDacl": (c_hit_if_null, 5, - "Never create NULL ACLs; an attacker can set it to Everyone (Deny All Access), " + + "Never create NULL ACLs; an attacker can set it to Everyone (Deny All Access), " "which would even forbid administrator access (CWE-732)", "", "misc", "", {'check_for_null': 3}), @@ -1672,7 +1671,7 @@ def display_header(): if output_format: print( '') + '"http://www.w3.org/TR/html4/loose.dtd">') print("") print("") print('')