Bug 1391711 - Implement a static analysis checker to detect usages of fopen/ open like functions on win32 platform. r?mystor draft
authorAndi-Bogdan Postelnicu <bpostelnicu@mozilla.com>
Wed, 20 Sep 2017 15:30:05 +0300
changeset 674103 a28ac360cc17f4aeb1cf5074723f49f2f8416271
parent 673329 97efdde466f18cf580fda9673cf4c38ee21fc7b7
child 734233 5c7d00adfa973e3dc68af0797e0b5ee908d97ad1
push id82732
push userbmo:bpostelnicu@mozilla.com
push dateTue, 03 Oct 2017 08:51:46 +0000
reviewersmystor
bugs1391711
milestone58.0a1
Bug 1391711 - Implement a static analysis checker to detect usages of fopen/ open like functions on win32 platform. r?mystor MozReview-Commit-ID: KjJWqgBaZz1
build/clang-plugin/Checks.inc
build/clang-plugin/ChecksIncludes.inc
build/clang-plugin/CustomMatchers.h
build/clang-plugin/FopenUssageChecker.cpp
build/clang-plugin/FopenUssageChecker.h
build/clang-plugin/moz.build
build/clang-plugin/tests/TestFopenUssage.cpp
build/clang-plugin/tests/moz.build
--- a/build/clang-plugin/Checks.inc
+++ b/build/clang-plugin/Checks.inc
@@ -5,16 +5,19 @@
 // The list of checker classes that are compatible with clang-tidy.
 
 CHECK(ArithmeticArgChecker, "arithmetic-argument")
 CHECK(AssertAssignmentChecker, "assignment-in-assert")
 CHECK(CanRunScriptChecker, "can-run-script")
 CHECK(DanglingOnTemporaryChecker, "dangling-on-temporary")
 CHECK(ExplicitImplicitChecker, "implicit-constructor")
 CHECK(ExplicitOperatorBoolChecker, "explicit-operator-bool")
+#ifdef WIN32
+CHECK(FopenUssageChecker, "fopen-usage")
+#endif
 CHECK(KungFuDeathGripChecker, "kungfu-death-grip")
 CHECK(MustOverrideChecker, "must-override")
 CHECK(MustReturnFromCallerChecker, "must-return-from-caller")
 CHECK(MustUseChecker, "must-use")
 CHECK(NaNExprChecker, "nan-expr")
 CHECK(NeedsNoVTableTypeChecker, "needs-no-vtable-type")
 CHECK(NoAddRefReleaseOnReturnChecker, "no-addref-release-on-return")
 CHECK(NoAutoTypeChecker, "no-auto-type")
--- a/build/clang-plugin/ChecksIncludes.inc
+++ b/build/clang-plugin/ChecksIncludes.inc
@@ -6,16 +6,19 @@
 // are compatible with clang-tidy.
 
 #include "ArithmeticArgChecker.h"
 #include "AssertAssignmentChecker.h"
 #include "CanRunScriptChecker.h"
 #include "DanglingOnTemporaryChecker.h"
 #include "ExplicitImplicitChecker.h"
 #include "ExplicitOperatorBoolChecker.h"
+#ifdef WIN32
+#include "FopenUssageChecker.h"
+#endif
 #include "KungFuDeathGripChecker.h"
 #include "MustOverrideChecker.h"
 #include "MustReturnFromCallerChecker.h"
 #include "MustUseChecker.h"
 #include "NaNExprChecker.h"
 #include "NeedsNoVTableTypeChecker.h"
 #include "NoAddRefReleaseOnReturnChecker.h"
 #include "NoAutoTypeChecker.h"
--- a/build/clang-plugin/CustomMatchers.h
+++ b/build/clang-plugin/CustomMatchers.h
@@ -40,16 +40,24 @@ AST_MATCHER(CXXMethodDecl, isRValueRefQu
 }
 
 AST_POLYMORPHIC_MATCHER(isFirstParty,                                          \
                         AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt)) {
   return !inThirdPartyPath(&Node, &Finder->getASTContext()) &&
          !ASTIsInSystemHeader(Finder->getASTContext(), Node);
 }
 
+/// This matcher will match locations in system headers.  This is adopted from
+/// isExpansionInSystemHeader in newer clangs, but modified in order to work
+/// with old clangs that we use on infra.
+AST_POLYMORPHIC_MATCHER(isInSystemHeader,                                      \
+                        AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt)) {
+  return ASTIsInSystemHeader(Finder->getASTContext(), Node);
+}
+
 /// This matcher will match temporary expressions.
 /// We need this matcher for compatibility with clang 3.* (clang 4 and above
 /// insert a MaterializeTemporaryExpr everywhere).
 AST_MATCHER(Expr, isTemporary) {
   return Node.isRValue() || Node.isXValue() ||
          isa<MaterializeTemporaryExpr>(&Node);
 }
 
@@ -102,23 +110,16 @@ AST_MATCHER(BinaryOperator, binaryEquali
 AST_MATCHER(BinaryOperator, binaryCommaOperator) {
   BinaryOperatorKind OpCode = Node.getOpcode();
   return OpCode == BO_Comma;
 }
 
 /// This matcher will match floating point types.
 AST_MATCHER(QualType, isFloat) { return Node->isRealFloatingType(); }
 
-/// This matcher will match locations in system headers.  This is adopted from
-/// isExpansionInSystemHeader in newer clangs, but modified in order to work
-/// with old clangs that we use on infra.
-AST_MATCHER(BinaryOperator, isInSystemHeader) {
-  return ASTIsInSystemHeader(Finder->getASTContext(), Node);
-}
-
 /// This matcher will match a list of files.  These files contain
 /// known NaN-testing expressions which we would like to whitelist.
 AST_MATCHER(BinaryOperator, isInWhitelistForNaNExpr) {
   const char *whitelist[] = {"SkScalar.h", "json_writer.cpp", "State.cpp"};
 
   SourceLocation Loc = Node.getOperatorLoc();
   auto &SourceManager = Finder->getASTContext().getSourceManager();
   SmallString<1024> FileName = SourceManager.getFilename(Loc);
new file mode 100644
--- /dev/null
+++ b/build/clang-plugin/FopenUssageChecker.cpp
@@ -0,0 +1,67 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "FopenUssageChecker.h"
+#include "CustomMatchers.h"
+
+void FopenUssageChecker::registerMatchers(MatchFinder *AstMatcher) {
+
+  auto hasConstCharPtrParam = [](const unsigned int Position) {
+    return allOf(hasParameter(Position, hasType(pointsTo(isAnyCharacter()))),
+                 hasParameter(Position, hasType(pointsTo(isConstQualified()))));
+  };
+
+  auto hasParamOfType = [](const unsigned int Position, const char *Name) {
+    return hasParameter(Position, hasType(asString(Name)));
+  };
+
+  auto hasIntegerParam = [](const unsigned int Position) {
+    return hasParameter(Position, hasType(isInteger()));
+  };
+
+  AstMatcher->addMatcher(
+      callExpr(
+          allOf(
+              isFirstParty(),
+              callee(functionDecl(allOf(
+                  isInSystemHeader(),
+                  anyOf(
+                      allOf(anyOf(allOf(hasName("fopen"),
+                                        hasConstCharPtrParam(0)),
+                                  allOf(hasName("fopen_s"),
+                                        hasParameter(
+                                            0, hasType(pointsTo(pointsTo(
+                                                   asString("FILE"))))),
+                                        hasConstCharPtrParam(2))),
+                            hasConstCharPtrParam(1)),
+                      allOf(anyOf(hasName("open"),
+                                  allOf(hasName("_open"), hasIntegerParam(2)),
+                                  allOf(hasName("_sopen"), hasIntegerParam(3))),
+                            hasConstCharPtrParam(0), hasIntegerParam(1)),
+                      allOf(hasName("_sopen_s"),
+                            hasParameter(0, hasType(pointsTo(isInteger()))),
+                            hasConstCharPtrParam(1), hasIntegerParam(2),
+                            hasIntegerParam(3), hasIntegerParam(4)),
+                      allOf(hasName("OpenFile"), hasParamOfType(0, "LPCSTR"),
+                            hasParamOfType(1, "LPOFSTRUCT"),
+                            hasIntegerParam(2)),
+                      allOf(hasName("CreateFileA"), hasParamOfType(0, "LPCSTR"),
+                            hasIntegerParam(1), hasIntegerParam(2),
+                            hasParamOfType(3, "LPSECURITY_ATTRIBUTES"),
+                            hasIntegerParam(4), hasIntegerParam(5),
+                            hasParamOfType(6, "HANDLE"))))))))
+          .bind("funcCall"),
+      this);
+}
+
+void FopenUssageChecker::check(const MatchFinder::MatchResult &Result) {
+  const CallExpr *FuncCall = Result.Nodes.getNodeAs<CallExpr>("funcCall");
+
+  if (FuncCall) {
+    diag(FuncCall->getLocStart(),
+         "usage of ASCII file functions (such as %0) is forbidden on windows",
+         DiagnosticIDs::Error)
+        << FuncCall->getDirectCallee()->getName();
+  }
+}
new file mode 100644
--- /dev/null
+++ b/build/clang-plugin/FopenUssageChecker.h
@@ -0,0 +1,19 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef FopenUssageChecker_h__
+#define FopenUssageChecker_h__
+
+#include "plugin.h"
+
+class FopenUssageChecker : public BaseCheck {
+public:
+  FopenUssageChecker(StringRef CheckName,
+                     ContextType *Context = nullptr)
+    : BaseCheck(CheckName, Context) {}
+  void registerMatchers(MatchFinder* AstMatcher) override;
+  void check(const MatchFinder::MatchResult &Result) override;
+};
+
+#endif
--- a/build/clang-plugin/moz.build
+++ b/build/clang-plugin/moz.build
@@ -36,16 +36,21 @@ UNIFIED_SOURCES += [
     'RefCountedCopyConstructorChecker.cpp',
     'RefCountedInsideLambdaChecker.cpp',
     'ScopeChecker.cpp',
     'SprintfLiteralChecker.cpp',
     'TrivialCtorDtorChecker.cpp',
     'VariableUsageHelpers.cpp',
 ]
 
+if CONFIG['OS_ARCH'] == 'WINNT':
+    UNIFIED_SOURCES += [
+        'FopenUssageChecker.cpp',
+    ]
+
 GENERATED_FILES += ['ThirdPartyPaths.cpp']
 third_party_paths = GENERATED_FILES['ThirdPartyPaths.cpp']
 third_party_paths.script = "ThirdPartyPaths.py:generate"
 third_party_paths.inputs = [
     '/tools/rewriting/ThirdPartyPaths.txt',
 ]
 
 DisableStlWrapping()
new file mode 100644
--- /dev/null
+++ b/build/clang-plugin/tests/TestFopenUssage.cpp
@@ -0,0 +1,34 @@
+#include <stdio.h>
+#include <io.h>
+#include <fcntl.h>
+#include <windows.h>
+
+void func_fopen() {
+  FILE *f1 = fopen("dummy.txt", "rt"); // expected-error {{usage of ASCII file functions (such as fopen) is forbidden on windows}}
+  FILE *f2;
+  fopen_s(&f2, "dummy.txt", "rt"); // expected-error {{usage of ASCII file functions (such as fopen_s) is forbidden on windows}}
+
+  int fh1 = _open("dummy.txt", _O_RDONLY); // expected-error {{usage of ASCII file functions (such as _open) is forbidden on windows}}
+  int fh2 = open("dummy.txt", _O_RDONLY); // expected-error {{usage of ASCII file functions (such as open) is forbidden on windows}}
+  int fh3 = _sopen("dummy.txt", _O_RDONLY, _SH_DENYRW); // expected-error {{usage of ASCII file functions (such as _sopen) is forbidden on windows}}
+  int fd4;
+  errno_t err = _sopen_s(&fd4, "dummy.txt", _O_RDONLY, _SH_DENYRW, 0); // expected-error {{usage of ASCII file functions (such as _sopen_s) is forbidden on windows}}
+
+  LPOFSTRUCT buffer;
+  HFILE hFile1 = OpenFile("dummy.txt", buffer, OF_READ); // expected-error {{usage of ASCII file functions (such as OpenFile) is forbidden on windows}}
+
+#ifndef UNICODE
+  // CreateFile is just an alias of CreateFileA
+  LPCSTR buffer2;
+  HANDLE hFile2 = CreateFile(buffer2, GENERIC_WRITE, 0, NULL, CREATE_NEW, // expected-error {{usage of ASCII file functions (such as CreateFileA) is forbidden on windows}}
+	  FILE_ATTRIBUTE_NORMAL, NULL);
+#else
+  // CreateFile is just an alias of CreateFileW and should not be matched
+  LPCWSTR buffer2;
+  HANDLE hFile2 = CreateFile(buffer2, GENERIC_WRITE, 0, NULL, CREATE_NEW,
+	  FILE_ATTRIBUTE_NORMAL, NULL);
+  LPCSTR buffer3;
+  HANDLE hFile3 = CreateFileA(buffer3, GENERIC_WRITE, 0, NULL, CREATE_NEW, // expected-error {{usage of ASCII file functions (such as CreateFileA) is forbidden on windows}}
+	  FILE_ATTRIBUTE_NORMAL, NULL);
+#endif
+}
--- a/build/clang-plugin/tests/moz.build
+++ b/build/clang-plugin/tests/moz.build
@@ -40,10 +40,15 @@ SOURCES += [
     'TestOverrideBaseCall.cpp',
     'TestOverrideBaseCallAnnotation.cpp',
     'TestRefCountedCopyConstructor.cpp',
     'TestSprintfLiteral.cpp',
     'TestStackClass.cpp',
     'TestTrivialCtorDtor.cpp',
 ]
 
+if CONFIG['OS_ARCH'] == 'WINNT':
+    UNIFIED_SOURCES += [
+        'TestFopenUssage.cpp',
+    ]
+
 DisableStlWrapping()
 NoVisibilityFlags()