[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel added a comment. In D35068#1441601 , @lebedev.ri wrote: > In D35068#1441581 , @koldaniel wrote: > > > Bug fixing: faulty handling of built-in functions. > > > Please open a new differential, that is a

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

lebedev.ri added a comment. In D35068#1441581 , @koldaniel wrote: > Bug fixing: faulty handling of built-in functions. Please open a new differential, that is a completely new patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel updated this revision to Diff 192120. koldaniel added a comment. Bug fixing: faulty handling of built-in functions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/ https://reviews.llvm.org/D35068 Files: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

NoQ added a comment. In D35068#1440830 , @koldaniel wrote: > In D35068#1438498 , @NoQ wrote: > > > There seems to be a crash in this code. @koldaniel, would you like to take > > a look?

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel added a comment. In D35068#1438498 , @NoQ wrote: > There seems to be a crash in this code. @koldaniel, would you like to take a > look? https://bugs.llvm.org/show_bug.cgi?id=41185 Hi, True, it is a faulty scenario, my question is what should

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

NoQ added a comment. Herald added a subscriber: Charusso. There seems to be a crash in this code. @koldaniel, would you like to take a look? https://bugs.llvm.org/show_bug.cgi?id=41185 Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

This revision was automatically updated to reflect the committed changes. Closed by commit rL353698: [analyzer] New checker for detecting usages of unsafe I/O functions (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

Szelethus accepted this revision. Szelethus added a comment. This looks great, thanks! I'll commit sometime tomorrow. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/ https://reviews.llvm.org/D35068 ___ cfe-commits mailing list

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel updated this revision to Diff 186131. koldaniel added a comment. Rebased. Documentation added. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/ https://reviews.llvm.org/D35068 Files: docs/analyzer/checkers.rst include/clang/StaticAnalyzer/Checkers/Checkers.td

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel added a comment. In D35068#1391758 , @Szelethus wrote: > Hmmm, `DescFile` was removed months ago from `Checkers.td`, are you sure you > uploaded the correct diff? No, you are absolutely right, an earlier diff had been uploaded, I will correct

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

Szelethus added a comment. Hmmm, `DescFile` was removed months ago from `Checkers.td`, are you sure you uploaded the correct diff? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/ https://reviews.llvm.org/D35068 ___ cfe-commits

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel updated this revision to Diff 186115. koldaniel added a comment. Rebased. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/ https://reviews.llvm.org/D35068 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

Szelethus added a comment. Could you rebase please? Many things have changed since you last update. After that I'll happily commit on your behalf if you don't have a commit access just yet. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/ https://reviews.llvm.org/D35068

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

NoQ accepted this revision. NoQ added a comment. Ok! I hope that the C11 check would do the trick, let's see how it goes :) In D35068#1364947 , @xazax.hun wrote: > What do we want to validate here? The lack of crashes? Or evaluate false > positive

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Yup, I'm sold on that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/ https://reviews.llvm.org/D35068 ___ cfe-commits

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

xazax.hun added a comment. To add an analogy, Clang Tidy will not require C++ Core Guidelines related checks to be evaluated on projects that are not following the guidelines as the results are meaningless for those projects. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D35068/new/

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

xazax.hun added a comment. In D35068#1361902 , @Szelethus wrote: > In D35068#1069880 , @koldaniel wrote: > > > I've evaluated this checker on LLVM+Clang, there were only a few (about 15) > > warnings, because of

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

whisperity added a comment. In D35068#1361902 , @Szelethus wrote: > Edit: it doesn't, but CMake is mostly a C project and it does! CMake really isn't a C project if you look at what language it actually uses - the C files come from tests and system

Re: [PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

Please also note that "safe" functions are C11 Annex K, and it is Optional. I believe glibc does not implement it, and has no intention of implementing it. By memory, i'm not sure any library other than Microsoft's does provide it. On Thu, Jan 17, 2019 at 9:17 PM Kristóf Umann via Phabricator

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

Szelethus requested changes to this revision. Szelethus added a comment. This revision now requires changes to proceed. In D35068#811436 , @NoQ wrote: > I wonder how noisy this check is - did you test it on large codebases? > Because these functions are

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

Szelethus added inline comments. Comment at: test/Analysis/security-syntax-checks.m:253 + FILE *file; + sprintf(buf, "a"); // expected-warning{{Call to function 'sprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

Szelethus accepted this revision. Szelethus added a comment. Overall I think this looks great, thanks! I left some inlines that would be nice to fix before commiting, but all of them are minor nits. Would it be possible for you to commit the clang-formatting and the actual logic separately?

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Herald added subscribers: donat.nagy, Szelethus, rnkovacs, baloghadamsoftware. LGTM! Any objections to commit this? I think this is quiet coding guideline specific check which is useful

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel added a comment. Herald added a subscriber: mikhail.ramalho. Hi, could you please take a look at this issue? https://reviews.llvm.org/D35068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel added a comment. Do you have any other comments? Could this checker be delivered into `security`? https://reviews.llvm.org/D35068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel added a comment. In https://reviews.llvm.org/D35068#1049530, @george.karpenkov wrote: > @koldaniel Have you evaluated this checker? On which codebases? Were the > warnings real security issues, or were they mostly spurious? The code seems > fine, but I'm not sure whether it should be

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

george.karpenkov added a comment. @koldaniel Have you evaluated this checker? On which codebases? Were the warnings real security issues, or were they mostly spurious? The code seems fine, but I'm not sure whether it should be in `security` or in `alpha`. https://reviews.llvm.org/D35068

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel added a comment. Do you have any other comments, or could this change be accepted? https://reviews.llvm.org/D35068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel updated this revision to Diff 137162. https://reviews.llvm.org/D35068 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp test/Analysis/security-syntax-checks.m Index: test/Analysis/security-syntax-checks.m

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:618 + StringRef Name = FD->getIdentifier()->getName(); + int ArgIndex = llvm::StringSwitch(Name) + .Case("sprintf", 1) koldaniel

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:618 + StringRef Name = FD->getIdentifier()->getName(); + int ArgIndex = llvm::StringSwitch(Name) + .Case("sprintf", 1) george.karpenkov

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

george.karpenkov added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:100 void checkCall_strcat(const CallExpr *CE, const FunctionDecl *FD); + void checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE, const FunctionDecl *FD);

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel updated this revision to Diff 136261. Herald added a subscriber: a.sidorin. Herald added a reviewer: george.karpenkov. https://reviews.llvm.org/D35068 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:382 +DescFile<"CheckSecuritySyntaxOnly.cpp">; + def DeprecatedBufferHandling : Checker<"DeprecatedBufferHandling">, +HelpText<"Warn on uses of deprecated buffer manipulating

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

xazax.hun added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:382 +DescFile<"CheckSecuritySyntaxOnly.cpp">; + def DeprecatedBufferHandling : Checker<"DeprecatedBufferHandling">, +HelpText<"Warn on uses of deprecated buffer manipulating

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:382 +DescFile<"CheckSecuritySyntaxOnly.cpp">; + def DeprecatedBufferHandling : Checker<"DeprecatedBufferHandling">, +HelpText<"Warn on uses of deprecated buffer manipulating

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

xazax.hun added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:382 +DescFile<"CheckSecuritySyntaxOnly.cpp">; + def DeprecatedBufferHandling : Checker<"DeprecatedBufferHandling">, +HelpText<"Warn on uses of deprecated buffer manipulating

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel updated this revision to Diff 117782. Herald added a subscriber: szepet. https://reviews.llvm.org/D35068 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/Driver/ToolChains/Clang.cpp lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel updated this revision to Diff 113094. koldaniel added a comment. Renaming the unsafe checker, updating tests. https://reviews.llvm.org/D35068 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/Driver/Tools.cpp lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel updated this revision to Diff 113065. koldaniel added a comment. Updated checker name, minor modifications https://reviews.llvm.org/D35068 Files: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

koldaniel marked 4 inline comments as done. koldaniel added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:597-598 + + if(!BR.getContext().getLangOpts().C11) +return; + NoQ wrote: > Note that you cannot easily figure

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

xazax.hun added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:622 +} +//===--===// +// Check: Use of 'sprintf', 'vsprintf', 'scanf', 'wscanf', 'fscanf', I

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

xazax.hun added a comment. In https://reviews.llvm.org/D35068#811437, @NoQ wrote: > It'd look good in clang-tidy (especially if extended to provide fixits), but > if Daniel is interested in having this feature in the analyzer (and picked by > clang-tidy from there), i wouldn't mind. > > I

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

NoQ added a comment. It'd look good in clang-tidy, but if Daniel is interested in having this feature in the analyzer (and picked by clang-tidy from there), i wouldn't mind. I wonder how noisy this check is - did you test it on large codebases? Because these functions are popular, and in many