Merge pull request #102 from david-a-wheeler/improvements2

Strip leading ./ from SARIF artifact URIs for uriBaseId portability

Author: david-a-wheeler

ChangeLog

@@ -58,6 +58,50 @@
 	* Removed duplicate word in help text.
 	* Fixed missing closing </li> tags in HTML output.
 	* Fixed typos in documentation.
+	* Fix false positives for method and member calls. Functions like
+	  read(), system(), or StrCat() are no longer flagged when preceded
+	  by "." or "->", e.g. stream.read(), obj.system(), ptr->read().
+	  Global calls and explicit global-scope calls (::system()) are still
+	  flagged. Fixes https://github.com/david-a-wheeler/flawfinder/issues/87
+	  https://github.com/david-a-wheeler/flawfinder/issues/83
+	  https://github.com/david-a-wheeler/flawfinder/issues/82
+	  https://github.com/david-a-wheeler/flawfinder/issues/76
+	  https://github.com/david-a-wheeler/flawfinder/issues/59
+	* Warn and skip files with encoding errors instead of aborting the
+	  entire scan. A count of skipped files is reported at the end and
+	  flawfinder exits with code 15 if any were skipped. The new
+	  --error-on-encoding flag restores the old abort-on-first-error
+	  behavior for strict pipelines.
+	  Fixes https://github.com/david-a-wheeler/flawfinder/issues/80
+	  https://github.com/david-a-wheeler/flawfinder/issues/68
+	* Fix SARIF output to produce valid URIs and helpUri values.
+	  File paths are now percent-encoded so spaces and special characters
+	  don't produce malformed URIs. Leading "./" prefixes are stripped so
+	  paths work correctly with the uriBaseId/SRCROOT placeholder (paths
+	  beginning with "-" retain "./" to avoid misinterpretation as CLI
+	  options). The helpUri field now reliably resolves to a CWE URL or
+	  falls back to a flawfinder rule URL; previously it could produce
+	  invalid values like ")".
+	  Fixes https://github.com/david-a-wheeler/flawfinder/issues/78
+	  https://github.com/david-a-wheeler/flawfinder/issues/67
+	  https://github.com/david-a-wheeler/flawfinder/issues/66
+	* Fix swprintf/vswprintf format-string detection. These functions take
+	  (buf, n, format, ...) so the format string is at position 3, not 2.
+	  Previously flawfinder inspected the size argument n instead, causing
+	  false positives on constant formats and missing non-constant ones.
+	  Fixes https://github.com/david-a-wheeler/flawfinder/issues/43
+	* Fix false positive: function names inside __attribute__((...)) are
+	  compiler metadata, not callable expressions,
+	  and are no longer flagged.
+	  E.g., __attribute__((format(printf, 1, 2))) no longer triggers a
+	  format-string warning.
+	  Fixes https://github.com/david-a-wheeler/flawfinder/issues/27
+	* Add --exclude PATTERN option (repeatable) to skip files or
+	  directories whose full path matches a glob pattern, e.g.
+	  --exclude '*/third_party/*'. Useful for ignoring generated code,
+	  vendored libraries, or test directories.
+	  Fixes https://github.com/david-a-wheeler/flawfinder/issues/89
+	  https://github.com/david-a-wheeler/flawfinder/issues/65
 
 2021-08-29 David A. Wheeler
 	* Version 2.0.19

flawfinder.py

@@ -362,7 +362,12 @@ def _to_sarif_rank(level):
 
     @staticmethod
     def _to_uri_path(path):
-        return url_quote(path.replace("\\", "/"), safe='/')
+        p = path.replace("\\", "/")
+        while p.startswith("./"):
+            p = p[2:]
+        if p.startswith("-"):
+            p = "./" + p  # prevent misinterpretation as a CLI option
+        return url_quote(p, safe='/')
 
     @staticmethod
     def _append_period(text):
@@ -647,7 +652,8 @@ def __init__(self, data):
         self.end = None
         self.parameters = None
         self.lookahead = None  # set only when extract_lookahead is true
-        _allowed_keys = {'check_for_null', 'extract_lookahead', 'format_position', 'input'}
+        _allowed_keys = {'check_for_null', 'extract_lookahead',
+                         'format_position', 'input', 'source_position'}
         for key in other:
             if key in _allowed_keys:
                 setattr(self, key, other[key])
@@ -1354,12 +1360,19 @@ def found_system(hit):
      "Use fgets() instead", "buffer", "", {'input': 1}, "FF1014"),
 
     # The "sprintf" hook will raise "format" issues instead if appropriate:
-    "sprintf|vsprintf|swprintf|vswprintf|_stprintf|_vstprintf":
+    "sprintf|vsprintf|_stprintf|_vstprintf":
     (c_sprintf, 4,
      "Does not check for buffer overflows (CWE-120)",
      "Use sprintf_s, snprintf, or vsnprintf",
      "buffer", "", {}, "FF1015"),
 
+    # swprintf/vswprintf take (buf, n, format, ...) so format is at position 3.
+    "swprintf|vswprintf":
+    (c_sprintf, 4,
+     "Does not check for buffer overflows (CWE-120)",
+     "Use sprintf_s, snprintf, or vsnprintf",
+     "buffer", "", {'source_position': 3}, "FF1015"),
+
     "printf|vprintf|vwprintf|vfwprintf|_vtprintf|wprintf":
     (c_printf, 4,
      "If format strings can be influenced by an attacker, they can be exploited (CWE-134)",
@@ -1881,6 +1894,33 @@ def _preceded_by_member_or_namespace(text, startpos):
     return False
 
 
+def _skip_attribute_args(text, pos):
+    """Advance past the ((...)) argument of __attribute__, tracking paren depth.
+
+    Returns the position after the closing paren, or pos unchanged if no
+    opening paren is found (malformed or no argument).
+    """
+    n = len(text)
+    while pos < n and text[pos] in ' \t\n\r':
+        pos += 1
+    if pos >= n or text[pos] != '(':
+        return pos
+    # Limitation: paren counting ignores string literals, so an unbalanced
+    # '(' inside a string argument (e.g. deprecated("old open( call")) will
+    # cause the scanner to skip the rest of the file.  This is accepted as a
+    # known edge case; attribute strings with unbalanced parens are very rare.
+    depth = 0
+    while pos < n:
+        if text[pos] == '(':
+            depth += 1
+        elif text[pos] == ')':
+            depth -= 1
+            if depth == 0:
+                return pos + 1
+        pos += 1
+    return pos
+
+
 def process_directive():
     "Given a directive, process it."
     global ignoreline, num_ignored_hits
@@ -2102,7 +2142,9 @@ def process_c_file(f, patch_infos):
                     i = endpos
                     word = text[startpos:endpos]
                     # print "Word is:", text[startpos:endpos]
-                    if (word in c_ruleset) and \
+                    if word == "__attribute__":
+                        i = _skip_attribute_args(text, endpos)
+                    elif (word in c_ruleset) and \
                        not _preceded_by_member_or_namespace(text, startpos) and \
                        c_valid_match(text, endpos):
                         if ((patch_infos is None)

test/correct-results-014.txt

@@ -0,0 +1,3 @@
+test-attribute.c:18:5:  [4] (format) printf:
+  If format strings can be influenced by an attacker, they can be exploited
+  (CWE-134). Use a constant for the format specification.

test/makefile

@@ -97,6 +97,14 @@ test_013: $(FLAWFINDER) test-member-calls.cpp
 	  test-member-calls.cpp > test-results-013.txt
 	@diff -u correct-results-013.txt test-results-013.txt
 
+test_014: $(FLAWFINDER) test-attribute.c
+	@echo 'test_014 (__attribute__((format(printf,...))) not flagged; real calls still flagged)'
+# printf inside __attribute__((...)) is a specifier keyword, not a call;
+# must produce no hit. Real printf calls outside attributes must still fire.
+	@$(PYTHON) $(FLAWFINDER) --omittime -m0 -DC --quiet \
+	  test-attribute.c > test-results-014.txt
+	@diff -u correct-results-014.txt test-results-014.txt
+
 test_012: $(FLAWFINDER) test-modern-cpp.cpp
 	@echo 'test_012 (modern C++ lexing: binary/hex literals, hex floats, digit separators)'
 # Modern C++ syntax must produce zero hits (no false positives from the lexer).
@@ -109,7 +117,7 @@ test_012: $(FLAWFINDER) test-modern-cpp.cpp
 # If everything works as expected, it just prints test numbers.
 # Set PYTHON as needed, including to ""
 test: test_001 test_002 test_003 test_004 test_005 test_006 test_007 test_008 \
-	  test_009 test_010 test_011 test_012 test_013
+	  test_009 test_010 test_011 test_012 test_013 test_014
 	@echo 'All tests pass!'
 
 check: test

test/test-attribute.c

@@ -0,0 +1,19 @@
+// test-attribute.c: __attribute__((format(printf,...))) must not produce
+// false positives; real printf calls outside attributes must still be flagged.
+
+// --- These should produce NO hits ---
+
+// GCC format attribute: 'printf' here is a specifier keyword, not a call.
+#define ZT_FORMAT_PRINTF(a, b) __attribute__((format(printf, a, b)))
+
+// Attribute directly on a declaration.
+void my_log(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
+
+// Nested parens inside attribute argument.
+void my_warn(const char *fmt, ...) __attribute__((format(printf, 1, 2), noinline));
+
+// --- These SHOULD still produce hits ---
+
+void real_call(const char *name) {
+    printf(name);  // non-constant format: real hit
+}