Merge rc/1.19 into next.

Author: adityasharad

change-notes/1.19/analysis-cpp.md

@@ -8,7 +8,7 @@
 | Cast from `char*` to `wchar_t*` (`cpp/incorrect-string-type-conversion`) | security, external/cwe/cwe-704 | Detects potentially dangerous casts from `char*` to `wchar_t*`.  Results are shown on LGTM by default. |
 | Dead code due to `goto` or `break` statement (`cpp/dead-code-goto`) | maintainability, external/cwe/cwe-561 | Detects dead code following a `goto` or `break` statement. Results are shown on LGTM by default. |
 | Inconsistent direction of for loop (`cpp/inconsistent-loop-direction`) | correctness, external/cwe/cwe-835 | Detects `for` loops where the increment and guard condition don't appear to correspond.  Results are shown on LGTM by default. |
-| Incorrect Not Operator Usage (`cpp/incorrect-not-operator-usage`) | security, external/cwe/cwe-480 | Finds uses of the logical not (`!`) operator that look like they should be bit-wise not (`~`).  Results are hidden on LGTM by default. |
+| Incorrect 'not' operator usage (`cpp/incorrect-not-operator-usage`) | security, external/cwe/cwe-480 | Finds uses of the logical not (`!`) operator that look like they should be bit-wise not (`~`).  Results are hidden on LGTM by default. |
 | Non-virtual destructor in base class (`cpp/virtual-destructor`) | reliability, readability, language-features | This query, `NonVirtualDestructorInBaseClass.ql`, is a replacement in LGTM for the query: No virtual destructor (`AV Rule 78.ql`). The new query ignores base classes with non-public destructors since we consider those to be adequately protected. The new version retains the query identifier, `cpp/virtual-destructor`, and results are displayed by default on LGTM. The old query is no longer run on LGTM. |
 | `NULL` application name with an unquoted path in call to `CreateProcess` (`cpp/unsafe-create-process-call`) | security, external/cwe/cwe-428 | Finds unsafe uses of the `CreateProcess` function.  Results are hidden on LGTM by default. |
 | Setting a DACL to `NULL` in a `SECURITY_DESCRIPTOR` (`cpp/unsafe-dacl-security-descriptor`) | security, external/cwe/cwe-732 | Finds code that creates world-writable objects on Windows by setting their DACL to `NULL`. Results are shown on LGTM by default. |

change-notes/1.19/analysis-javascript.md

@@ -28,7 +28,7 @@ to run queries and explore the data flow in results.
 | Replacement of a substring with itself (`js/identity-replacement`) | correctness, security, external/cwe/cwe-116 | Highlights string replacements that replace a string with itself, which usually indicates a mistake. Results shown on LGTM by default. |
 | Stored cross-site scripting (`js/stored-xss`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights uncontrolled stored values flowing into HTML content, indicating a potential violation of [CWE-079](https://cwe.mitre.org/data/definitions/79.html). Results shown on LGTM by default. |
 | Unclear precedence of nested operators (`js/unclear-operator-precedence`) | maintainability, correctness, external/cwe/cwe-783 | Highlights nested binary operators whose relative precedence is easy to misunderstand. Results shown on LGTM by default. |
-| Unneeded defensive code | correctness, external/cwe/cwe-570, external/cwe/cwe-571 | Highlights locations where defensive code is not needed. Results are shown on LGTM by default. |
+| Unneeded defensive code (`js/unneeded-defensive-code`) | correctness, external/cwe/cwe-570, external/cwe/cwe-571 | Highlights locations where defensive code is not needed. Results are shown on LGTM by default. |
 | Unsafe dynamic method access (`js/unsafe-dynamic-method-access` ) | security, external/cwe/cwe-094 | Highlights code that invokes a user-controlled method on an object with unsafe methods. Results are shown on LGTM by default. |
 | Unvalidated dynamic method access (`js/unvalidated-dynamic-method-call` ) | security, external/cwe/cwe-754 | Highlights code that invokes a user-controlled method without guarding against exceptional circumstances. Results are shown on LGTM by default. |
 | Useless assignment to property (`js/useless-assignment-to-property`) | maintainability | Highlights property assignments whose value is always overwritten. Results are shown on LGTM by default. |

cpp/ql/src/Likely Bugs/Likely Typos/IncorrectNotOperatorUsage.ql

@@ -1,5 +1,5 @@
 /**
- * @name Incorrect Not Operator Usage
+ * @name Incorrect 'not' operator usage
  * @description Usage of a logical-not (!) operator as an operand for a bit-wise operation.
  *              This commonly indicates the usage of an incorrect operator instead of the bit-wise not (~) operator,
  *              also known as ones' complement operator.

csharp/autobuilder/Semmle.Autobuild.Tests/BuildScripts.cs

@@ -7,7 +7,7 @@ namespace Semmle.Autobuild
     /// </summary>
     class AspBuildRule : IBuildRule
     {
-        public BuildScript Analyse(Autobuilder builder)
+        public BuildScript Analyse(Autobuilder builder, bool auto)
         {
             var command = new CommandBuilder(builder.Actions).
                 RunCommand(builder.Actions.PathCombine(builder.SemmleJavaHome, "bin", "java")).

csharp/autobuilder/Semmle.Autobuild/AspBuildRule.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Linq;
+using System.Text.RegularExpressions;
 
 namespace Semmle.Autobuild
 {
@@ -37,14 +38,14 @@ public void ReadEnvironment(IBuildActions actions)
         {
             RootDirectory = actions.GetCurrentDirectory();
             VsToolsVersion = actions.GetEnvironmentVariable(prefix + "VSTOOLS_VERSION");
-            MsBuildArguments = actions.GetEnvironmentVariable(prefix + "MSBUILD_ARGUMENTS");
+            MsBuildArguments = actions.GetEnvironmentVariable(prefix + "MSBUILD_ARGUMENTS").AsStringWithExpandedEnvVars(actions);
             MsBuildPlatform = actions.GetEnvironmentVariable(prefix + "MSBUILD_PLATFORM");
             MsBuildConfiguration = actions.GetEnvironmentVariable(prefix + "MSBUILD_CONFIGURATION");
             MsBuildTarget = actions.GetEnvironmentVariable(prefix + "MSBUILD_TARGET");
-            DotNetArguments = actions.GetEnvironmentVariable(prefix + "DOTNET_ARGUMENTS");
+            DotNetArguments = actions.GetEnvironmentVariable(prefix + "DOTNET_ARGUMENTS").AsStringWithExpandedEnvVars(actions);
             DotNetVersion = actions.GetEnvironmentVariable(prefix + "DOTNET_VERSION");
             BuildCommand = actions.GetEnvironmentVariable(prefix + "BUILD_COMMAND");
-            Solution = actions.GetEnvironmentVariable(prefix + "SOLUTION").AsList(new string[0]);
+            Solution = actions.GetEnvironmentVariable(prefix + "SOLUTION").AsListWithExpandedEnvVars(actions, new string[0]);
 
             IgnoreErrors = actions.GetEnvironmentVariable(prefix + "IGNORE_ERRORS").AsBool("ignore_errors", false);
             Buildless = actions.GetEnvironmentVariable(prefix + "BUILDLESS").AsBool("buildless", false);
@@ -92,12 +93,29 @@ public static Language AsLanguage(this string key)
             }
         }
 
-        public static string[] AsList(this string value, string[] defaultValue)
+        public static string[] AsListWithExpandedEnvVars(this string value, IBuildActions actions, string[] defaultValue)
         {
             if (value == null)
                 return defaultValue;
 
-            return value.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).ToArray();
+            return value.
+                Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).
+                Select(s => AsStringWithExpandedEnvVars(s, actions)).ToArray();
+        }
+
+        static readonly Regex linuxEnvRegEx = new Regex(@"\$([a-zA-Z_][a-zA-Z_0-9]*)", RegexOptions.Compiled);
+
+        public static string AsStringWithExpandedEnvVars(this string value, IBuildActions actions)
+        {
+            if (string.IsNullOrEmpty(value))
+                return value;
+
+            // `Environment.ExpandEnvironmentVariables` only works with Windows-style
+            // environment variables
+            var windowsStyle = actions.IsWindows()
+                ? value
+                : linuxEnvRegEx.Replace(value, m => $"%{m.Groups[1].Value}%");
+            return actions.EnvironmentExpandEnvironmentVariables(windowsStyle);
         }
     }
 }

csharp/autobuilder/Semmle.Autobuild/AutobuildOptions.cs

@@ -16,7 +16,8 @@ interface IBuildRule
         /// Analyse the files and produce a build script.
         /// </summary>
         /// <param name="builder">The files and options relating to the build.</param>
-        BuildScript Analyse(Autobuilder builder);
+        /// <param name="auto">Whether this build rule is being automatically applied.</param>
+        BuildScript Analyse(Autobuilder builder, bool auto);
     }
 
     /// <summary>
@@ -135,9 +136,9 @@ public Autobuilder(IBuildActions actions, AutobuildOptions options)
                     foreach (var solution in options.Solution)
                     {
                         if (actions.FileExists(solution))
-                            ret.Add(new Solution(this, solution));
+                            ret.Add(new Solution(this, solution, true));
                         else
-                            Log(Severity.Error, $"The specified solution file {solution} was not found");
+                            Log(Severity.Error, $"The specified project or solution file {solution} was not found");
                     }
                     return ret;
                 }
@@ -172,7 +173,7 @@ IEnumerable<IProjectOrSolution> FindFiles(string extension, Func<string, Project
                     return ret;
 
                 // Then look for `.sln` files
-                ret = FindFiles(".sln", f => new Solution(this, f))?.ToList();
+                ret = FindFiles(".sln", f => new Solution(this, f, false))?.ToList();
                 if (ret != null)
                     return ret;
 
@@ -250,17 +251,17 @@ BuildScript CheckExtractorRun(bool warnOnFailure) =>
             switch (GetCSharpBuildStrategy())
             {
                 case CSharpBuildStrategy.CustomBuildCommand:
-                    attempt = new BuildCommandRule().Analyse(this) & CheckExtractorRun(true);
+                    attempt = new BuildCommandRule().Analyse(this, false) & CheckExtractorRun(true);
                     break;
                 case CSharpBuildStrategy.Buildless:
                     // No need to check that the extractor has been executed in buildless mode
-                    attempt = new StandaloneBuildRule().Analyse(this);
+                    attempt = new StandaloneBuildRule().Analyse(this, false);
                     break;
                 case CSharpBuildStrategy.MSBuild:
-                    attempt = new MsBuildRule().Analyse(this) & CheckExtractorRun(true);
+                    attempt = new MsBuildRule().Analyse(this, false) & CheckExtractorRun(true);
                     break;
                 case CSharpBuildStrategy.DotNet:
-                    attempt = new DotNetRule().Analyse(this) & CheckExtractorRun(true);
+                    attempt = new DotNetRule().Analyse(this, false) & CheckExtractorRun(true);
                     break;
                 case CSharpBuildStrategy.Auto:
                     var cleanTrapFolder =
@@ -285,20 +286,20 @@ BuildScript IntermediateAttempt(BuildScript s) =>
 
                     attempt =
                         // First try .NET Core
-                        IntermediateAttempt(new DotNetRule().Analyse(this)) |
+                        IntermediateAttempt(new DotNetRule().Analyse(this, true)) |
                         // Then MSBuild
-                        (() => IntermediateAttempt(new MsBuildRule().Analyse(this))) |
+                        (() => IntermediateAttempt(new MsBuildRule().Analyse(this, true))) |
                         // And finally look for a script that might be a build script
-                        (() => new BuildCommandAutoRule().Analyse(this) & CheckExtractorRun(true)) |
+                        (() => new BuildCommandAutoRule().Analyse(this, true) & CheckExtractorRun(true)) |
                         // All attempts failed: print message
                         AutobuildFailure();
                     break;
             }
 
             return
                 attempt &
-                (() => new AspBuildRule().Analyse(this)) &
-                (() => new XmlBuildRule().Analyse(this));
+                (() => new AspBuildRule().Analyse(this, false)) &
+                (() => new XmlBuildRule().Analyse(this, false));
         }
 
         /// <summary>
@@ -337,13 +338,13 @@ enum CSharpBuildStrategy
         BuildScript GetCppBuildScript()
         {
             if (Options.BuildCommand != null)
-                return new BuildCommandRule().Analyse(this);
+                return new BuildCommandRule().Analyse(this, false);
 
             return
                 // First try MSBuild
-                new MsBuildRule().Analyse(this) |
+                new MsBuildRule().Analyse(this, true) |
                 // Then look for a script that might be a build script
-                (() => new BuildCommandAutoRule().Analyse(this)) |
+                (() => new BuildCommandAutoRule().Analyse(this, true)) |
                 // All attempts failed: print message
                 AutobuildFailure();
         }

csharp/autobuilder/Semmle.Autobuild/Autobuilder.cs

@@ -113,6 +113,12 @@ public interface IBuildActions
         /// Loads the XML document from <paramref name="filename"/>.
         /// </summary>
         XmlDocument LoadXml(string filename);
+
+        /// <summary>
+        /// Expand all Windows-style environment variables in <paramref name="s"/>,
+        /// Environment.ExpandEnvironmentVariables()
+        /// </summary>
+        string EnvironmentExpandEnvironmentVariables(string s);
     }
 
     /// <summary>
@@ -187,6 +193,8 @@ XmlDocument IBuildActions.LoadXml(string filename)
 
         string IBuildActions.GetFullPath(string path) => Path.GetFullPath(path);
 
+        public string EnvironmentExpandEnvironmentVariables(string s) => Environment.ExpandEnvironmentVariables(s);
+
         public static readonly IBuildActions Instance = new SystemBuildActions();
     }
 }

csharp/autobuilder/Semmle.Autobuild/BuildActions.cs

@@ -25,7 +25,7 @@ class BuildCommandAutoRule : IBuildRule
             "build"
         };
 
-        public BuildScript Analyse(Autobuilder builder)
+        public BuildScript Analyse(Autobuilder builder, bool auto)
         {
             builder.Log(Severity.Info, "Attempting to locate build script");
 

csharp/autobuilder/Semmle.Autobuild/BuildCommandAutoRule.cs

@@ -5,7 +5,7 @@
     /// </summary>
     class BuildCommandRule : IBuildRule
     {
-        public BuildScript Analyse(Autobuilder builder)
+        public BuildScript Analyse(Autobuilder builder, bool auto)
         {
             if (builder.Options.BuildCommand == null)
                 return BuildScript.Failure;