diff --git a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql index 0a52a2b0ff4..4bc58eb0854 100644 --- a/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql +++ b/cpp/ql/src/Likely Bugs/Leap Year/UncheckedLeapYearAfterYearModification.ql @@ -227,6 +227,30 @@ class IgnorableUnaryBitwiseOperation extends IgnorableOperation instanceof Unary class IgnorableAssignmentBitwiseOperation extends IgnorableOperation instanceof AssignBitwiseOperation { } +class YearFieldAssignmentNode extends DataFlow::Node { + YearFieldAccess access; + + YearFieldAssignmentNode() { + exists(Function f | + f = this.getEnclosingCallable().getUnderlyingCallable() and not f instanceof IgnorableFunction + | + this.asDefinition().(Assignment).getLValue() = access + or + this.asDefinition().(CrementOperation).getOperand() = access + or + exists(Call c | c.getAnArgument() = access and this.asDefiningArgument() = access) + or + exists(Call c, AddressOfExpr aoe | + c.getAnArgument() = aoe and + aoe.getOperand() = access and + this.asDefiningArgument() = aoe + ) + ) + } + + YearFieldAccess getYearFieldAccess() { result = access } +} + /** * An arithmetic operation where one of the operands is a pointer or char type, ignore it */ @@ -287,24 +311,7 @@ predicate isOperationSourceCandidate(Expr e) { } /** - * A data flow that tracks an ignorable operation (such as a bitwise operation) to an operation source, so we may disqualify it. - */ -module IgnorableOperationToOperationSourceCandidateConfig implements DataFlow::ConfigSig { - predicate isSource(DataFlow::Node n) { n.asExpr() instanceof IgnorableOperation } - - predicate isSink(DataFlow::Node n) { isOperationSourceCandidate(n.asExpr()) } - - // looking for sources and sinks in the same function - DataFlow::FlowFeature getAFeature() { - result instanceof DataFlow::FeatureEqualSourceSinkCallContext - } -} - -module IgnorableOperationToOperationSourceCandidateFlow = - TaintTracking::Global; - -/** - * The set of all expressions which is a candidate expression and also does not flow from to to some ignorable expression (eg. bitwise op) + * The set of all expressions that are candidate expression. * ``` * a = something <<< 2; * myDate.year = a + 1; // invalid @@ -314,49 +321,16 @@ module IgnorableOperationToOperationSourceCandidateFlow = * ``` */ class OperationSource extends Expr { - OperationSource() { - isOperationSourceCandidate(this) and - // If the candidate came from an ignorable operation, ignore the candidate - // NOTE: we cannot easily flow the candidate to an ignorable operation as that can - // be tricky in practice, e.g., a mod operation on a year would be part of a leap year check - // but a mod operation ending in a year is more indicative of something to ignore (a conversion) - not exists(IgnorableOperationToOperationSourceCandidateFlow::PathNode sink | - sink.getNode().asExpr() = this and - sink.isSink() - ) - } -} - -class YearFieldAssignmentNode extends DataFlow::Node { - YearFieldAccess access; - - YearFieldAssignmentNode() { - exists(Function f | - f = this.getEnclosingCallable().getUnderlyingCallable() and not f instanceof IgnorableFunction - ) and - ( - this.asDefinition().(Assignment).getLValue() = access - or - this.asDefinition().(CrementOperation).getOperand() = access - or - exists(Call c | c.getAnArgument() = access and this.asDefiningArgument() = access) - or - exists(Call c, AddressOfExpr aoe | - c.getAnArgument() = aoe and - aoe.getOperand() = access and - this.asDefiningArgument() = aoe - ) - ) - } - - YearFieldAccess getYearFieldAccess() { result = access } + OperationSource() { isOperationSourceCandidate(this) } } /** - * A DataFlow configuration for identifying flows from an identified source - * to the Year field of a date object. + * An initial DataFlow configuration for identifying flows from an identified source + * to the Year field of a date object. This is used to restrict the sinks of + * `IgnorableOperationToOperationSourceCandidateConfig` and the sinks of the + * final `OperationToYearAssignmentConfig`. */ -module OperationToYearAssignmentConfig implements DataFlow::ConfigSig { +module OperationToYearAssignmentConfig0 implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node n) { n.asExpr() instanceof OperationSource } predicate isSink(DataFlow::Node n) { @@ -411,6 +385,62 @@ module OperationToYearAssignmentConfig implements DataFlow::ConfigSig { predicate isBarrierOut(DataFlow::Node n) { isSink(n) } } +module OperationToYearAssignmentFlow0 = TaintTracking::Global; + +predicate yearAssignmentFlowsFromSource(DataFlow::Node source, DataFlow::Node sink) { + OperationToYearAssignmentFlow0::flow(source, sink) +} + +/** + * A data flow that tracks an ignorable operation (such as a bitwise operation) to an operation source, so we may disqualify it. + * Sinks are restricted to operation source candidates that have a flow to a year assignment in `OperationToYearAssignmentFlow0`. + */ +module IgnorableOperationToOperationSourceCandidateConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node n) { n.asExpr() instanceof IgnorableOperation } + + predicate isSink(DataFlow::Node n) { + isOperationSourceCandidate(n.asExpr()) and + yearAssignmentFlowsFromSource(n, _) + } + + // looking for sources and sinks in the same function + DataFlow::FlowFeature getAFeature() { + result instanceof DataFlow::FeatureEqualSourceSinkCallContext + } +} + +module IgnorableOperationToOperationSourceCandidateFlow = + TaintTracking::Global; + +/** + * The final DataFlow configuration that refines `OperationToYearAssignmentConfig0` by + * additionally filtering out operation sources that flow from an ignorable operation + * (via `IgnorableOperationToOperationSourceCandidateFlow`). + */ +module OperationToYearAssignmentConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node n) { yearAssignmentFlowsFromSource(n, _) } + + predicate isSink(DataFlow::Node n) { + exists(DataFlow::Node operation | + yearAssignmentFlowsFromSource(operation, n) and + // If the candidate came from an ignorable operation, ignore the candidate + // NOTE: we cannot easily flow the candidate to an ignorable operation as that can + // be tricky in practice, e.g., a mod operation on a year would be part of a leap year check + // but a mod operation ending in a year is more indicative of something to ignore (a conversion) + not exists(IgnorableOperationToOperationSourceCandidateFlow::PathNode sink | + sink.getNode() = operation and + sink.isSink() + ) + ) + } + + predicate isBarrier(DataFlow::Node n) { OperationToYearAssignmentConfig0::isBarrier(n) } + + predicate isBarrierIn(DataFlow::Node n) { isSource(n) } + + predicate isBarrierOut(DataFlow::Node n) { isSink(n) } +} + module OperationToYearAssignmentFlow = TaintTracking::Global; predicate isLeapYearCheckSink(DataFlow::Node sink) { diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs index 9d3d79e4c4f..699e06d273c 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs @@ -95,9 +95,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching args += " /p:EnableWindowsTargeting=true"; } - if (restoreSettings.ExtraArgs is not null) + if (restoreSettings.NugetSources is not null) { - args += $" {restoreSettings.ExtraArgs}"; + args += $" {restoreSettings.NugetSources}"; } return args; diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs index eec6a2b8d3b..d14dee50652 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/IDotNet.cs @@ -17,7 +17,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching IList GetNugetFeedsFromFolder(string folderPath); } - public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, string? ExtraArgs = null, string? PathToNugetConfig = null, bool ForceReevaluation = false, bool TargetWindows = false); + public record class RestoreSettings(string File, string PackageDirectory, bool ForceDotnetRefAssemblyFetching, string? NugetSources = null, string? PathToNugetConfig = null, bool ForceReevaluation = false, bool TargetWindows = false); public partial record class RestoreResult(bool Success, IList Output) { @@ -33,6 +33,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private readonly Lazy hasNugetNoStablePackageVersionError = new(() => Output.Any(s => s.Contains("NU1103"))); public bool HasNugetNoStablePackageVersionError => hasNugetNoStablePackageVersionError.Value; + private readonly Lazy hasNugetPackageMissingError = new(() => Output.Any(s => s.Contains("NU1101"))); + public bool HasNugetPackageMissingError => hasNugetPackageMissingError.Value; + private static IEnumerable GetFirstGroupOnMatch(Regex regex, IEnumerable lines) => lines .Select(line => regex.Match(line)) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs index b90b388e865..e97b0b118c6 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs @@ -33,7 +33,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching /// /// Create the package manager for a specified source tree. /// - public NugetExeWrapper(FileProvider fileProvider, DependencyDirectory packageDirectory, Semmle.Util.Logging.ILogger logger) + public NugetExeWrapper(FileProvider fileProvider, DependencyDirectory packageDirectory, Semmle.Util.Logging.ILogger logger, Func useDefaultFeed) { this.fileProvider = fileProvider; this.packageDirectory = packageDirectory; @@ -43,7 +43,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching { logger.LogInfo($"Found packages.config files, trying to use nuget.exe for package restore"); nugetExe = ResolveNugetExe(); - if (HasNoPackageSource()) + if (HasNoPackageSource() && useDefaultFeed()) { // We only modify or add a top level nuget.config file nugetConfigPath = Path.Combine(fileProvider.SourceDir.FullName, "nuget.config"); diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index 1d01412ee05..e042285af11 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Collections.Immutable; using System.IO; using System.Linq; using System.Net; @@ -27,8 +28,12 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private readonly IDiagnosticsWriter diagnosticsWriter; private readonly DependencyDirectory legacyPackageDirectory; private readonly DependencyDirectory missingPackageDirectory; + private readonly DependencyDirectory emptyPackageDirectory; private readonly ILogger logger; private readonly ICompilationInfoContainer compilationInfoContainer; + private readonly bool checkNugetFeedResponsiveness = EnvironmentVariables.GetBooleanOptOut(EnvironmentVariableNames.CheckNugetFeedResponsiveness); + private readonly ImmutableHashSet privateRegistryFeeds; + private readonly bool hasPrivateRegistryFeeds; public DependencyDirectory PackageDirectory { get; } @@ -45,6 +50,8 @@ namespace Semmle.Extraction.CSharp.DependencyFetching this.fileContent = fileContent; this.dotnet = dotnet; this.dependabotProxy = dependabotProxy; + this.privateRegistryFeeds = dependabotProxy?.RegistryURLs.ToImmutableHashSet() ?? []; + this.hasPrivateRegistryFeeds = privateRegistryFeeds.Count > 0; this.diagnosticsWriter = diagnosticsWriter; this.logger = logger; this.compilationInfoContainer = compilationInfoContainer; @@ -52,6 +59,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching PackageDirectory = new DependencyDirectory("packages", "package", logger); legacyPackageDirectory = new DependencyDirectory("legacypackages", "legacy package", logger); missingPackageDirectory = new DependencyDirectory("missingpackages", "missing package", logger); + emptyPackageDirectory = new DependencyDirectory("empty", "empty package", logger); } public string? TryRestore(string package) @@ -110,25 +118,50 @@ namespace Semmle.Extraction.CSharp.DependencyFetching public HashSet Restore() { var assemblyLookupLocations = new HashSet(); - var checkNugetFeedResponsiveness = EnvironmentVariables.GetBooleanOptOut(EnvironmentVariableNames.CheckNugetFeedResponsiveness); logger.LogInfo($"Checking NuGet feed responsiveness: {checkNugetFeedResponsiveness}"); compilationInfoContainer.CompilationInfos.Add(("NuGet feed responsiveness checked", checkNugetFeedResponsiveness ? "1" : "0")); - HashSet? explicitFeeds = null; - HashSet? allFeeds = null; + HashSet explicitFeeds = []; + HashSet reachableFeeds = []; try { - if (checkNugetFeedResponsiveness && !CheckFeeds(out explicitFeeds, out allFeeds)) + // Find feeds that are configured in NuGet.config files and divide them into ones that + // are explicitly configured for the project or by a private registry, and "all feeds" + // (including inherited ones) from other locations on the host outside of the working directory. + (explicitFeeds, var allFeeds) = GetAllFeeds(); + + if (checkNugetFeedResponsiveness) { - // todo: we could also check the reachability of the inherited nuget feeds, but to use those in the fallback we would need to handle authentication too. - var unresponsiveMissingPackageLocation = DownloadMissingPackagesFromSpecificFeeds([], explicitFeeds); - return unresponsiveMissingPackageLocation is null - ? [] - : [unresponsiveMissingPackageLocation]; + var inheritedFeeds = allFeeds.Except(explicitFeeds).ToHashSet(); + + if (inheritedFeeds.Count > 0) + { + compilationInfoContainer.CompilationInfos.Add(("Inherited NuGet feed count", inheritedFeeds.Count.ToString())); + } + + var timeout = CheckSpecifiedFeeds(explicitFeeds, out var reachableExplicitFeeds); + reachableFeeds.UnionWith(reachableExplicitFeeds); + + var allExplicitReachable = explicitFeeds.Count == reachableExplicitFeeds.Count; + EmitUnreachableFeedsDiagnostics(allExplicitReachable); + + if (timeout) + { + // If we experience a timeout, we use this fallback. + // todo: we could also check the reachability of the inherited nuget feeds, but to use those in the fallback we would need to handle authentication too. + var unresponsiveMissingPackageLocation = DownloadMissingPackagesFromSpecificFeeds([], explicitFeeds); + return unresponsiveMissingPackageLocation is null + ? [] + : [unresponsiveMissingPackageLocation]; + } + + // Inherited feeds should only be used, if they are indeed reachable (as they may be environment specific). + CheckSpecifiedFeeds(inheritedFeeds, out var reachableInheritedFeeds); + reachableFeeds.UnionWith(reachableInheritedFeeds); } - using (var nuget = new NugetExeWrapper(fileProvider, legacyPackageDirectory, logger)) + using (var nuget = new NugetExeWrapper(fileProvider, legacyPackageDirectory, logger, IsDefaultFeedReachable)) { var count = nuget.InstallPackages(); @@ -167,9 +200,10 @@ namespace Semmle.Extraction.CSharp.DependencyFetching logger.LogError($"Failed to restore NuGet packages with nuget.exe: {exc.Message}"); } - var restoredProjects = RestoreSolutions(out var container); + // Restore project dependencies with `dotnet restore`. + var restoredProjects = RestoreSolutions(reachableFeeds, out var container); var projects = fileProvider.Projects.Except(restoredProjects); - RestoreProjects(projects, allFeeds, out var containers); + RestoreProjects(projects, reachableFeeds, out var containers); var dependencies = containers.Flatten(container); @@ -192,6 +226,53 @@ namespace Semmle.Extraction.CSharp.DependencyFetching return assemblyLookupLocations; } + /// + /// Tests which of the feeds given by are reachable. + /// + /// The feeds to check. + /// Whether the feeds are fallback feeds or not. + /// Whether a timeout occurred while checking the feeds. + /// The list of feeds that could be reached. + private List GetReachableNuGetFeeds(HashSet feedsToCheck, bool isFallback, out bool isTimeout) + { + var fallbackStr = isFallback ? "fallback " : ""; + logger.LogInfo($"Checking {fallbackStr}NuGet feed reachability on feeds: {string.Join(", ", feedsToCheck.OrderBy(f => f))}"); + + var (initialTimeout, tryCount) = GetFeedRequestSettings(isFallback); + var timeout = false; + var reachableFeeds = feedsToCheck + .Where(feed => + { + var reachable = IsFeedReachable(feed, initialTimeout, tryCount, out var feedTimeout); + timeout |= feedTimeout; + return reachable; + }) + .ToList(); + + if (reachableFeeds.Count == 0) + { + logger.LogWarning($"No {fallbackStr}NuGet feeds are reachable."); + } + else + { + logger.LogInfo($"Reachable {fallbackStr}NuGet feeds: {string.Join(", ", reachableFeeds.OrderBy(f => f))}"); + } + + isTimeout = timeout; + return reachableFeeds; + } + + private bool IsDefaultFeedReachable() + { + if (checkNugetFeedResponsiveness) + { + var (initialTimeout, tryCount) = GetFeedRequestSettings(isFallback: false); + return IsFeedReachable(PublicNugetOrgFeed, initialTimeout, tryCount, out var _); + } + + return true; + } + private List GetReachableFallbackNugetFeeds(HashSet? feedsFromNugetConfigs) { var fallbackFeeds = EnvironmentVariables.GetURLs(EnvironmentVariableNames.FallbackNugetFeeds).ToHashSet(); @@ -212,17 +293,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - logger.LogInfo($"Checking fallback NuGet feed reachability on feeds: {string.Join(", ", fallbackFeeds.OrderBy(f => f))}"); - var (initialTimeout, tryCount) = GetFeedRequestSettings(isFallback: true); - var reachableFallbackFeeds = fallbackFeeds.Where(feed => IsFeedReachable(feed, initialTimeout, tryCount, allowExceptions: false)).ToList(); - if (reachableFallbackFeeds.Count == 0) - { - logger.LogWarning("No fallback NuGet feeds are reachable."); - } - else - { - logger.LogInfo($"Reachable fallback NuGet feeds: {string.Join(", ", reachableFallbackFeeds.OrderBy(f => f))}"); - } + var reachableFallbackFeeds = GetReachableNuGetFeeds(fallbackFeeds, isFallback: true, out var _); compilationInfoContainer.CompilationInfos.Add(("Reachable fallback NuGet feed count", reachableFallbackFeeds.Count.ToString())); @@ -237,10 +308,12 @@ namespace Semmle.Extraction.CSharp.DependencyFetching /// Populates dependencies with the relevant dependencies from the assets files generated by the restore. /// Returns a list of projects that are up to date with respect to restore. /// - private IEnumerable RestoreSolutions(out DependencyContainer dependencies) + private IEnumerable RestoreSolutions(HashSet reachableFeeds, out DependencyContainer dependencies) { var successCount = 0; var nugetSourceFailures = 0; + var nugetMissingPackageFailures = 0; + var assets = new Assets(logger); var isWindows = fileContent.UseWindowsForms || fileContent.UseWpf; @@ -248,7 +321,8 @@ namespace Semmle.Extraction.CSharp.DependencyFetching var projects = fileProvider.Solutions.SelectMany(solution => { logger.LogInfo($"Restoring solution {solution}..."); - var res = dotnet.Restore(new(solution, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, TargetWindows: isWindows)); + var nugetSources = MakeRestoreSourcesArgument(solution, reachableFeeds); + var res = dotnet.Restore(new(solution, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, NugetSources: nugetSources, TargetWindows: isWindows)); if (res.Success) { successCount++; @@ -257,51 +331,84 @@ namespace Semmle.Extraction.CSharp.DependencyFetching { nugetSourceFailures++; } + if (res.HasNugetPackageMissingError) + { + nugetMissingPackageFailures++; + } assets.AddDependenciesRange(res.AssetsFilePaths); return res.RestoredProjects; }).ToList(); dependencies = assets.Dependencies; compilationInfoContainer.CompilationInfos.Add(("Successfully restored solution files", successCount.ToString())); compilationInfoContainer.CompilationInfos.Add(("Failed solution restore with package source error", nugetSourceFailures.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Failed solution restore with missing package error", nugetMissingPackageFailures.ToString())); compilationInfoContainer.CompilationInfos.Add(("Restored projects through solution files", projects.Count.ToString())); return projects; } + private string FeedsToRestoreArgument(IEnumerable feeds) + { + // If there are no feeds, we want to override any default feeds that `dotnet restore` would use by passing a dummy source argument. + if (!feeds.Any()) + { + return $" -s \"{emptyPackageDirectory.DirInfo.FullName}\""; + } + + // Add package sources. If any are present, they override all sources specified in + // the configuration file(s). + var feedArgs = new StringBuilder(); + foreach (var feed in feeds) + { + feedArgs.Append($" -s \"{feed}\""); + } + + return feedArgs.ToString(); + } + + /// + /// Constructs the list of NuGet sources to use for this restore. + /// (1) Use the feeds we get from `dotnet nuget list source` + /// (2) Use private registries, if they are configured + /// + /// Path to project/solution + /// The set of reachable NuGet feeds. + /// A string representing the NuGet sources argument for the restore command. + private string? MakeRestoreSourcesArgument(string path, HashSet reachableFeeds) + { + // Do not construct an set of explicit NuGet sources to use for restore. + if (!checkNugetFeedResponsiveness && !hasPrivateRegistryFeeds) + { + return null; + } + + // Find the path specific feeds. + var folder = GetDirectoryName(path); + var feedsToConsider = folder is not null ? GetFeeds(() => dotnet.GetNugetFeedsFromFolder(folder)).ToHashSet() : []; + + if (hasPrivateRegistryFeeds) + { + feedsToConsider.UnionWith(privateRegistryFeeds); + } + + var feedsToUse = checkNugetFeedResponsiveness + ? feedsToConsider.Where(reachableFeeds.Contains) + : feedsToConsider; + + return FeedsToRestoreArgument(feedsToUse); + } + /// /// Executes `dotnet restore` on all projects in projects. /// This is done in parallel for performance reasons. /// Populates dependencies with the relative paths to the assets files generated by the restore. /// /// A list of paths to project files. - private void RestoreProjects(IEnumerable projects, HashSet? configuredSources, out ConcurrentBag dependencies) + /// The set of reachable NuGet feeds. + private void RestoreProjects(IEnumerable projects, HashSet reachableFeeds, out ConcurrentBag dependencies) { - // Conservatively, we only set this to a non-null value if a Dependabot proxy is enabled. - // This ensures that we continue to get the old behaviour where feeds are taken from - // `nuget.config` files instead of the command-line arguments. - string? extraArgs = null; - - if (this.dependabotProxy is not null) - { - // If the Dependabot proxy is configured, then our main goal is to make `dotnet` aware - // of the private registry feeds. However, since providing them as command-line arguments - // to `dotnet` ignores other feeds that may be configured, we also need to add the feeds - // we have discovered from analysing `nuget.config` files. - var sources = configuredSources ?? new(); - this.dependabotProxy.RegistryURLs.ForEach(url => sources.Add(url)); - - // Add package sources. If any are present, they override all sources specified in - // the configuration file(s). - var feedArgs = new StringBuilder(); - foreach (string source in sources) - { - feedArgs.Append($" -s {source}"); - } - - extraArgs = feedArgs.ToString(); - } - var successCount = 0; var nugetSourceFailures = 0; + var nugetMissingPackageFailures = 0; ConcurrentBag collectedDependencies = []; var isWindows = fileContent.UseWindowsForms || fileContent.UseWpf; @@ -314,7 +421,8 @@ namespace Semmle.Extraction.CSharp.DependencyFetching foreach (var project in projectGroup) { logger.LogInfo($"Restoring project {project}..."); - var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, extraArgs, TargetWindows: isWindows)); + var nugetSources = MakeRestoreSourcesArgument(project, reachableFeeds); + var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true, NugetSources: nugetSources, TargetWindows: isWindows)); assets.AddDependenciesRange(res.AssetsFilePaths); lock (sync) { @@ -326,6 +434,10 @@ namespace Semmle.Extraction.CSharp.DependencyFetching { nugetSourceFailures++; } + if (res.HasNugetPackageMissingError) + { + nugetMissingPackageFailures++; + } } } collectedDependencies.Add(assets.Dependencies); @@ -333,6 +445,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching dependencies = collectedDependencies; compilationInfoContainer.CompilationInfos.Add(("Successfully restored project files", successCount.ToString())); compilationInfoContainer.CompilationInfos.Add(("Failed project restore with package source error", nugetSourceFailures.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Failed project restore with missing package error", nugetMissingPackageFailures.ToString())); } private AssemblyLookupLocation? DownloadMissingPackagesFromSpecificFeeds(IEnumerable usedPackageNames, HashSet? feedsFromNugetConfigs) @@ -623,28 +736,22 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - private static async Task ExecuteGetRequest(string address, HttpClient httpClient, CancellationToken cancellationToken) + private static async Task ExecuteGetRequest(string address, HttpClient httpClient, CancellationToken cancellationToken) { - using var stream = await httpClient.GetStreamAsync(address, cancellationToken); - var buffer = new byte[1024]; - int bytesRead; - while ((bytesRead = stream.Read(buffer, 0, buffer.Length)) > 0) - { - // do nothing - } + return await httpClient.GetAsync(address, HttpCompletionOption.ResponseHeadersRead, cancellationToken); } - private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount, bool allowExceptions = true) + private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount, out bool isTimeout) { logger.LogInfo($"Checking if NuGet feed '{feed}' is reachable..."); // Configure the HttpClient to be aware of the Dependabot Proxy, if used. HttpClientHandler httpClientHandler = new(); - if (this.dependabotProxy != null) + if (dependabotProxy != null) { - httpClientHandler.Proxy = new WebProxy(this.dependabotProxy.Address); + httpClientHandler.Proxy = new WebProxy(dependabotProxy.Address); - if (this.dependabotProxy.Certificate != null) + if (dependabotProxy.Certificate != null) { httpClientHandler.ServerCertificateCustomValidationCallback = (message, cert, chain, _) => { @@ -659,7 +766,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching return false; } chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; - chain.ChainPolicy.CustomTrustStore.Add(this.dependabotProxy.Certificate); + chain.ChainPolicy.CustomTrustStore.Add(dependabotProxy.Certificate); return chain.Build(cert); }; } @@ -667,13 +774,17 @@ namespace Semmle.Extraction.CSharp.DependencyFetching using HttpClient client = new(httpClientHandler); + isTimeout = false; + for (var i = 0; i < tryCount; i++) { using var cts = new CancellationTokenSource(); cts.CancelAfter(timeoutMilliSeconds); try { - ExecuteGetRequest(feed, client, cts.Token).GetAwaiter().GetResult(); + logger.LogInfo($"Attempt {i + 1}/{tryCount} to reach NuGet feed '{feed}'."); + using var response = ExecuteGetRequest(feed, client, cts.Token).GetAwaiter().GetResult(); + response.EnsureSuccessStatusCode(); logger.LogInfo($"Querying NuGet feed '{feed}' succeeded."); return true; } @@ -688,14 +799,13 @@ namespace Semmle.Extraction.CSharp.DependencyFetching continue; } - // We're only interested in timeouts. - var start = allowExceptions ? "Considering" : "Not considering"; - logger.LogInfo($"Querying NuGet feed '{feed}' failed in a timely manner. {start} the feed for use. The reason for the failure: {exc.Message}"); - return allowExceptions; + logger.LogInfo($"Querying NuGet feed '{feed}' failed. The reason for the failure: {exc.Message}"); + return false; } } logger.LogWarning($"Didn't receive answer from NuGet feed '{feed}'. Tried it {tryCount} times."); + isTimeout = true; return false; } @@ -719,42 +829,10 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } /// - /// Checks that we can connect to all NuGet feeds that are explicitly configured in configuration files - /// as well as any private package registry feeds that are configured. + /// Retrieves a list of excluded NuGet feeds from the corresponding environment variable. /// - /// Outputs the set of explicit feeds. - /// Outputs the set of all feeds (explicit and inherited). - /// True if all feeds are reachable or false otherwise. - private bool CheckFeeds(out HashSet explicitFeeds, out HashSet allFeeds) + private HashSet GetExcludedFeeds() { - (explicitFeeds, allFeeds) = GetAllFeeds(); - HashSet feedsToCheck = explicitFeeds; - - // If private package registries are configured for C#, then check those - // in addition to the ones that are configured in `nuget.config` files. - this.dependabotProxy?.RegistryURLs.ForEach(url => feedsToCheck.Add(url)); - - var allFeedsReachable = this.CheckSpecifiedFeeds(feedsToCheck); - - var inheritedFeeds = allFeeds.Except(explicitFeeds).ToHashSet(); - if (inheritedFeeds.Count > 0) - { - logger.LogInfo($"Inherited NuGet feeds (not checked for reachability): {string.Join(", ", inheritedFeeds.OrderBy(f => f))}"); - compilationInfoContainer.CompilationInfos.Add(("Inherited NuGet feed count", inheritedFeeds.Count.ToString())); - } - - return allFeedsReachable; - } - - /// - /// Checks that we can connect to the specified NuGet feeds. - /// - /// The set of package feeds to check. - /// True if all feeds are reachable or false otherwise. - private bool CheckSpecifiedFeeds(HashSet feeds) - { - logger.LogInfo("Checking that NuGet feeds are reachable..."); - var excludedFeeds = EnvironmentVariables.GetURLs(EnvironmentVariableNames.ExcludedNugetFeedsFromResponsivenessCheck) .ToHashSet(); @@ -763,9 +841,49 @@ namespace Semmle.Extraction.CSharp.DependencyFetching logger.LogInfo($"Excluded NuGet feeds from responsiveness check: {string.Join(", ", excludedFeeds.OrderBy(f => f))}"); } - var (initialTimeout, tryCount) = GetFeedRequestSettings(isFallback: false); + return excludedFeeds; + } - var allFeedsReachable = feeds.All(feed => excludedFeeds.Contains(feed) || IsFeedReachable(feed, initialTimeout, tryCount)); + /// + /// Checks that we can connect to the specified NuGet feeds. + /// + /// The set of package feeds to check. + /// The list of feeds that were reachable. + /// + /// True if there is a timeout when trying to reach the feeds (excluding any feeds that are configured + /// to be excluded from the check) or false otherwise. + /// + private bool CheckSpecifiedFeeds(HashSet feeds, out HashSet reachableFeeds) + { + // Exclude any feeds from the feed check that are configured by the corresponding environment variable. + // These feeds are always assumed to be reachable. + var excludedFeeds = GetExcludedFeeds(); + + HashSet feedsToCheck = feeds.Where(feed => + { + if (excludedFeeds.Contains(feed)) + { + logger.LogInfo($"Not checking reachability of NuGet feed '{feed}' as it is in the list of excluded feeds."); + return false; + } + return true; + }).ToHashSet(); + + reachableFeeds = GetReachableNuGetFeeds(feedsToCheck, isFallback: false, out var isTimeout).ToHashSet(); + + // Always consider feeds excluded for the reachability check as reachable. + reachableFeeds.UnionWith(feeds.Where(feed => excludedFeeds.Contains(feed))); + + return isTimeout; + } + + /// + /// If is `false`, logs this and emits a diagnostic. + /// Adds a `CompilationInfos` entry either way. + /// + /// Whether all feeds were reachable or not. + private void EmitUnreachableFeedsDiagnostics(bool allFeedsReachable) + { if (!allFeedsReachable) { logger.LogWarning("Found unreachable NuGet feed in C# analysis with build-mode 'none'. This may cause missing dependencies in the analysis."); @@ -779,8 +897,6 @@ namespace Semmle.Extraction.CSharp.DependencyFetching )); } compilationInfoContainer.CompilationInfos.Add(("All NuGet feeds reachable", allFeedsReachable ? "1" : "0")); - - return allFeedsReachable; } private IEnumerable GetFeeds(Func> getNugetFeeds) @@ -811,6 +927,19 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } + private string? GetDirectoryName(string path) + { + try + { + return new FileInfo(path).Directory?.FullName; + } + catch (Exception exc) + { + logger.LogWarning($"Failed to get directory of '{path}': {exc}"); + } + return null; + } + private (HashSet explicitFeeds, HashSet allFeeds) GetAllFeeds() { var nugetConfigs = fileProvider.NugetConfigs; @@ -828,11 +957,11 @@ namespace Semmle.Extraction.CSharp.DependencyFetching if (invalidNugetConfigs.Count() > 0) { - this.logger.LogWarning(string.Format( + logger.LogWarning(string.Format( "Found incorrectly named NuGet configuration files: {0}", string.Join(", ", invalidNugetConfigs) )); - this.diagnosticsWriter.AddEntry(new DiagnosticMessage( + diagnosticsWriter.AddEntry(new DiagnosticMessage( Language.CSharp, "buildless/case-sensitive-nuget-config", "Found NuGet configuration files which are not correctly named", @@ -864,41 +993,33 @@ namespace Semmle.Extraction.CSharp.DependencyFetching logger.LogDebug("No NuGet feeds found in nuget.config files."); } - // todo: this could be improved. - HashSet? allFeeds = null; + // If private package registries are configured for C#, then consider those + // in addition to the ones that are configured in `nuget.config` files. + if (hasPrivateRegistryFeeds) + { + logger.LogInfo($"Found {privateRegistryFeeds.Count} private registry feeds configured for C#: {string.Join(", ", privateRegistryFeeds.OrderBy(f => f))}"); + explicitFeeds.UnionWith(privateRegistryFeeds); + } + + HashSet allFeeds = []; + + // Add all explicitFeeds to the set of all feeds. + allFeeds.UnionWith(explicitFeeds); + + // Obtain the list of feeds from the root source directory. + // If a NuGet file is present it will be respected, otherwise we will just get the machine/environment specific feeds. + var nugetFeedsFromRoot = GetFeeds(() => dotnet.GetNugetFeedsFromFolder(fileProvider.SourceDir.FullName)); + allFeeds.UnionWith(nugetFeedsFromRoot); if (nugetConfigs.Count > 0) { - // We don't have to get the feeds from each of the folders from below, it would be enought to check the folders that recursively contain the others. - allFeeds = nugetConfigs - .Select(config => - { - try - { - return new FileInfo(config).Directory?.FullName; - } - catch (Exception exc) - { - logger.LogWarning($"Failed to get directory of '{config}': {exc}"); - } - return null; - }) + var nugetConfigFeeds = nugetConfigs + .Select(GetDirectoryName) .Where(folder => folder != null) .SelectMany(folder => GetFeeds(() => dotnet.GetNugetFeedsFromFolder(folder!))) .ToHashSet(); - // If we have discovered any explicit feeds, then we also expect these to be in the set of all feeds. - // Normally, it is a safe assumption to make that `GetNugetFeedsFromFolder` will include the feeds configured - // in a NuGet configuration file in the given directory. There is one exception: on a system with case-sensitive - // file systems, we may discover a configuration file such as `Nuget.Config` which is not recognised by `dotnet nuget`. - // In that case, our call to `GetNugetFeeds` will retrieve the feeds from that file (because it is accepted when - // provided explicitly as `--configfile` argument), but the call to `GetNugetFeedsFromFolder` will not. - allFeeds.UnionWith(explicitFeeds); - } - else - { - // If we haven't found any `nuget.config` files, then obtain a list of feeds from the root source directory. - allFeeds = GetFeeds(() => dotnet.GetNugetFeedsFromFolder(this.fileProvider.SourceDir.FullName)).ToHashSet(); + allFeeds.UnionWith(nugetConfigFeeds); } logger.LogInfo($"Found {allFeeds.Count} NuGet feeds (with inherited ones) in nuget.config files: {string.Join(", ", allFeeds.OrderBy(f => f))}"); @@ -923,6 +1044,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching PackageDirectory?.Dispose(); legacyPackageDirectory?.Dispose(); missingPackageDirectory?.Dispose(); + emptyPackageDirectory?.Dispose(); } /// diff --git a/csharp/ql/integration-tests/all-platforms/standalone_resx/CompilationInfo.expected b/csharp/ql/integration-tests/all-platforms/standalone_resx/CompilationInfo.expected index dfab1016a6b..6d91b270022 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone_resx/CompilationInfo.expected +++ b/csharp/ql/integration-tests/all-platforms/standalone_resx/CompilationInfo.expected @@ -1,5 +1,7 @@ | All NuGet feeds reachable | 1.0 | +| Failed project restore with missing package error | 0.0 | | Failed project restore with package source error | 0.0 | +| Failed solution restore with missing package error | 0.0 | | Failed solution restore with package source error | 0.0 | | Inherited NuGet feed count | 1.0 | | NuGet feed responsiveness checked | 1.0 | diff --git a/csharp/ql/integration-tests/all-platforms/standalone_slnx/CompilationInfo.expected b/csharp/ql/integration-tests/all-platforms/standalone_slnx/CompilationInfo.expected index 3bd3941b27c..82cf0509d34 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone_slnx/CompilationInfo.expected +++ b/csharp/ql/integration-tests/all-platforms/standalone_slnx/CompilationInfo.expected @@ -1,5 +1,7 @@ | All NuGet feeds reachable | 1.0 | +| Failed project restore with missing package error | 0.0 | | Failed project restore with package source error | 0.0 | +| Failed solution restore with missing package error | 0.0 | | Failed solution restore with package source error | 0.0 | | Inherited NuGet feed count | 1.0 | | NuGet feed responsiveness checked | 1.0 | diff --git a/csharp/ql/integration-tests/all-platforms/standalone_winforms/CompilationInfo.expected b/csharp/ql/integration-tests/all-platforms/standalone_winforms/CompilationInfo.expected index 6b06566033c..63ddd4903f3 100644 --- a/csharp/ql/integration-tests/all-platforms/standalone_winforms/CompilationInfo.expected +++ b/csharp/ql/integration-tests/all-platforms/standalone_winforms/CompilationInfo.expected @@ -1,5 +1,7 @@ | All NuGet feeds reachable | 1.0 | +| Failed project restore with missing package error | 0.0 | | Failed project restore with package source error | 0.0 | +| Failed solution restore with missing package error | 0.0 | | Failed solution restore with package source error | 0.0 | | Inherited NuGet feed count | 1.0 | | NuGet feed responsiveness checked | 1.0 | diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/Assemblies.expected b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/Assemblies.expected new file mode 100644 index 00000000000..b676e41c184 --- /dev/null +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/Assemblies.expected @@ -0,0 +1 @@ +| test-db/working/packages/newtonsoft.json/13.0.4/lib/net6.0/Newtonsoft.Json.dll:0:0:0:0 | Newtonsoft.Json, Version=13.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed | diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/Assemblies.ql b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/Assemblies.ql new file mode 100644 index 00000000000..0eb33b7ae37 --- /dev/null +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/Assemblies.ql @@ -0,0 +1,5 @@ +import csharp + +from Assembly a +where exists(a.getFile().getAbsolutePath().indexOf("newtonsoft.json")) +select a diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/CompilationInfo.expected b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/CompilationInfo.expected new file mode 100644 index 00000000000..ff0b29da33f --- /dev/null +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/CompilationInfo.expected @@ -0,0 +1,22 @@ +| All NuGet feeds reachable | 1.0 | +| Failed project restore with missing package error | 0.0 | +| Failed project restore with package source error | 0.0 | +| Failed solution restore with missing package error | 0.0 | +| Failed solution restore with package source error | 0.0 | +| Inherited NuGet feed count | 1.0 | +| NuGet feed responsiveness checked | 1.0 | +| Project files on filesystem | 1.0 | +| Reachable fallback NuGet feed count | 1.0 | +| Resolved assembly conflicts | 0.0 | +| Resource extraction enabled | 0.0 | +| Restored .NET framework variants | 1.0 | +| Restored projects through solution files | 0.0 | +| Solution files on filesystem | 0.0 | +| Source files generated | 0.0 | +| Source files on filesystem | 1.0 | +| Successfully restored project files | 1.0 | +| Successfully restored solution files | 0.0 | +| Unresolved references | 0.0 | +| UseWPF set | 0.0 | +| UseWindowsForms set | 0.0 | +| WebView extraction enabled | 1.0 | diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/CompilationInfo.ql b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/CompilationInfo.ql new file mode 100644 index 00000000000..073ffe3b224 --- /dev/null +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/CompilationInfo.ql @@ -0,0 +1,15 @@ +import csharp +import semmle.code.csharp.commons.Diagnostics + +query predicate compilationInfo(string key, float value) { + key != "Resolved references" and + not key.matches("Compiler diagnostic count for%") and + exists(Compilation c, string infoKey, string infoValue | infoValue = c.getInfo(infoKey) | + key = infoKey and + value = infoValue.toFloat() + or + not exists(infoValue.toFloat()) and + key = infoKey + ": " + infoValue and + value = 1 + ) +} diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/clear/nuget.config b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/clear/nuget.config new file mode 100644 index 00000000000..a3f3aea61c8 --- /dev/null +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/clear/nuget.config @@ -0,0 +1,6 @@ + + + + + + diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/global.json b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/global.json new file mode 100644 index 00000000000..ed604974070 --- /dev/null +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/global.json @@ -0,0 +1,5 @@ +{ + "sdk": { + "version": "10.0.201" + } +} diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/proj/Program.cs b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/proj/Program.cs new file mode 100644 index 00000000000..c340f4c32fd --- /dev/null +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/proj/Program.cs @@ -0,0 +1 @@ +class Program { } diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/proj/proj.csproj b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/proj/proj.csproj new file mode 100644 index 00000000000..6010c6c7f37 --- /dev/null +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/proj/proj.csproj @@ -0,0 +1,16 @@ + + + + Exe + net10.0 + + + + + + + + + + + diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/test.py b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/test.py new file mode 100644 index 00000000000..cbbf2eb1d64 --- /dev/null +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_clear/test.py @@ -0,0 +1,12 @@ +import os +import runs_on + + +@runs_on.posix +def test(codeql, csharp): + # Making sure the reachability test of `nuget.org` succeeds: + os.environ["CODEQL_EXTRACTOR_CSHARP_BUILDLESS_NUGET_FEEDS_CHECK_FALLBACK_TIMEOUT"] = "1000" + os.environ["CODEQL_EXTRACTOR_CSHARP_BUILDLESS_NUGET_FEEDS_CHECK_FALLBACK_LIMIT"] = "5" + + # This test checks that the nuget.config file in the clear folder is not applied to the restore of the project. + codeql.database.create(build_mode="none") diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_config_error/CompilationInfo.expected b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_config_error/CompilationInfo.expected index 3a74bcbd56e..4acd4f54e8a 100644 --- a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_config_error/CompilationInfo.expected +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_config_error/CompilationInfo.expected @@ -1,7 +1,10 @@ -| All NuGet feeds reachable | 1.0 | -| Failed project restore with package source error | 1.0 | +| All NuGet feeds reachable | 0.0 | +| Failed project restore with missing package error | 1.0 | +| Failed project restore with package source error | 0.0 | +| Failed solution restore with missing package error | 0.0 | | Failed solution restore with package source error | 0.0 | | Fallback nuget restore | 1.0 | +| Inherited NuGet feed count | 1.0 | | NuGet feed responsiveness checked | 1.0 | | Project files on filesystem | 1.0 | | Reachable fallback NuGet feed count | 1.0 | diff --git a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_config_fallback/CompilationInfo.expected b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_config_fallback/CompilationInfo.expected index c53a17f0300..9cc03f2f537 100644 --- a/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_config_fallback/CompilationInfo.expected +++ b/csharp/ql/integration-tests/posix/standalone_dependencies_nuget_config_fallback/CompilationInfo.expected @@ -1,5 +1,6 @@ | All NuGet feeds reachable | 0.0 | | Fallback nuget restore | 1.0 | +| Inherited NuGet feed count | 1.0 | | NuGet feed responsiveness checked | 1.0 | | Project files on filesystem | 1.0 | | Reachable fallback NuGet feed count | 2.0 | diff --git a/csharp/ql/lib/change-notes/2026-04-10-nuget-feed-usage-in-bmn.md b/csharp/ql/lib/change-notes/2026-04-10-nuget-feed-usage-in-bmn.md new file mode 100644 index 00000000000..a4282d0468d --- /dev/null +++ b/csharp/ql/lib/change-notes/2026-04-10-nuget-feed-usage-in-bmn.md @@ -0,0 +1,4 @@ +--- +category: majorAnalysis +--- +* When resolving dependencies in `build-mode: none`, `dotnet restore` now explicitly receives reachable NuGet feeds configured in `nuget.config` when feed responsiveness checking is enabled (the default), and any private registries directly, improving reliability when default feeds are unavailable or restricted. diff --git a/java/ql/lib/change-notes/2026-04-04-sensitive-log-hash-sanitizer.md b/java/ql/lib/change-notes/2026-04-04-sensitive-log-hash-sanitizer.md new file mode 100644 index 00000000000..7323ab09737 --- /dev/null +++ b/java/ql/lib/change-notes/2026-04-04-sensitive-log-hash-sanitizer.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `java/sensitive-log` query now treats method calls whose names contain "encrypt", "hash", or "digest" as sanitizers, consistent with the existing treatment in `java/cleartext-storage-in-log`. This reduces false positives when sensitive data is hashed or encrypted before logging. diff --git a/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll b/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll index 21d82bef657..83f51f7eedf 100644 --- a/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll +++ b/java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll @@ -2,6 +2,7 @@ import java private import semmle.code.java.dataflow.TaintTracking +private import semmle.code.java.security.Sanitizers private import semmle.code.java.security.SensitiveActions /** A sink representing persistent storage that saves data in clear text. */ @@ -76,17 +77,6 @@ private class DefaultCleartextStorageSanitizer extends CleartextStorageSanitizer } } -/** - * Method call for encrypting sensitive information. As there are various implementations of - * encryption (reversible and non-reversible) from both JDK and third parties, this class simply - * checks method name to take a best guess to reduce false positives. - */ -private class EncryptedSensitiveMethodCall extends MethodCall { - EncryptedSensitiveMethodCall() { - this.getMethod().getName().toLowerCase().matches(["%encrypt%", "%hash%", "%digest%"]) - } -} - /** Flow configuration for encryption methods flowing to inputs of persistent storage. */ private module EncryptedValueFlowConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node src) { src.asExpr() instanceof EncryptedSensitiveMethodCall } diff --git a/java/ql/lib/semmle/code/java/security/Sanitizers.qll b/java/ql/lib/semmle/code/java/security/Sanitizers.qll index e00071da2d8..0c5f9b98070 100644 --- a/java/ql/lib/semmle/code/java/security/Sanitizers.qll +++ b/java/ql/lib/semmle/code/java/security/Sanitizers.qll @@ -63,3 +63,14 @@ class RegexpCheckBarrier extends DataFlow::Node { exists(RegexMatch rm | rm instanceof Annotation | this.asExpr() = rm.getString()) } } + +/** + * A method call for encrypting, hashing, or digesting sensitive information. As there are various + * implementations of encryption (reversible and non-reversible) from both JDK and third parties, + * this class simply checks the method name to take a best guess to reduce false positives. + */ +class EncryptedSensitiveMethodCall extends MethodCall { + EncryptedSensitiveMethodCall() { + this.getMethod().getName().toLowerCase().matches(["%encrypt%", "%hash%", "%digest%"]) + } +} diff --git a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll index 7058b844cbd..f35cae0e67f 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll @@ -120,6 +120,14 @@ private class DefaultSensitiveLoggerBarrier extends SensitiveLoggerBarrier { } } +/** + * A barrier for sensitive data that has been hashed, encrypted, or digested before logging. + * This is consistent with the treatment of encryption in `CleartextStorageQuery.qll` (CWE-312). + */ +private class EncryptionBarrier extends SensitiveLoggerBarrier { + EncryptionBarrier() { this.asExpr() instanceof EncryptedSensitiveMethodCall } +} + /** A data-flow configuration for identifying potentially-sensitive data flowing to a log output. */ module SensitiveLoggerConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof SensitiveLoggerSource } diff --git a/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected b/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected index 4a5ed058b50..1155e4fb991 100644 --- a/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected +++ b/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected @@ -3,14 +3,15 @@ | Test.java:12:22:12:52 | ... + ... | Test.java:12:44:12:52 | authToken : String | Test.java:12:22:12:52 | ... + ... | This $@ is written to a log file. | Test.java:12:44:12:52 | authToken | potentially sensitive information | | Test.java:21:22:21:75 | ... + ... | Test.java:21:44:21:52 | authToken : String | Test.java:21:22:21:75 | ... + ... | This $@ is written to a log file. | Test.java:21:44:21:52 | authToken | potentially sensitive information | | Test.java:22:22:22:75 | ... + ... | Test.java:22:44:22:52 | authToken : String | Test.java:22:22:22:75 | ... + ... | This $@ is written to a log file. | Test.java:22:44:22:52 | authToken | potentially sensitive information | -| Test.java:66:21:66:43 | ... + ... | Test.java:66:33:66:43 | accessToken : String | Test.java:66:21:66:43 | ... + ... | This $@ is written to a log file. | Test.java:66:33:66:43 | accessToken | potentially sensitive information | -| Test.java:67:21:67:45 | ... + ... | Test.java:67:34:67:45 | clientSecret : String | Test.java:67:21:67:45 | ... + ... | This $@ is written to a log file. | Test.java:67:34:67:45 | clientSecret | potentially sensitive information | -| Test.java:68:21:68:42 | ... + ... | Test.java:68:34:68:42 | apiSecret : String | Test.java:68:21:68:42 | ... + ... | This $@ is written to a log file. | Test.java:68:34:68:42 | apiSecret | potentially sensitive information | -| Test.java:69:21:69:44 | ... + ... | Test.java:69:33:69:44 | sessionToken : String | Test.java:69:21:69:44 | ... + ... | This $@ is written to a log file. | Test.java:69:33:69:44 | sessionToken | potentially sensitive information | -| Test.java:70:21:70:43 | ... + ... | Test.java:70:33:70:43 | bearerToken : String | Test.java:70:21:70:43 | ... + ... | This $@ is written to a log file. | Test.java:70:33:70:43 | bearerToken | potentially sensitive information | -| Test.java:71:21:71:39 | ... + ... | Test.java:71:31:71:39 | secretKey : String | Test.java:71:21:71:39 | ... + ... | This $@ is written to a log file. | Test.java:71:31:71:39 | secretKey | potentially sensitive information | -| Test.java:72:21:72:44 | ... + ... | Test.java:72:33:72:44 | refreshToken : String | Test.java:72:21:72:44 | ... + ... | This $@ is written to a log file. | Test.java:72:33:72:44 | refreshToken | potentially sensitive information | -| Test.java:73:21:73:43 | ... + ... | Test.java:73:33:73:43 | secretValue : String | Test.java:73:21:73:43 | ... + ... | This $@ is written to a log file. | Test.java:73:33:73:43 | secretValue | potentially sensitive information | +| Test.java:31:21:31:37 | ... + ... | Test.java:31:30:31:37 | password : String | Test.java:31:21:31:37 | ... + ... | This $@ is written to a log file. | Test.java:31:30:31:37 | password | potentially sensitive information | +| Test.java:75:21:75:43 | ... + ... | Test.java:75:33:75:43 | accessToken : String | Test.java:75:21:75:43 | ... + ... | This $@ is written to a log file. | Test.java:75:33:75:43 | accessToken | potentially sensitive information | +| Test.java:76:21:76:45 | ... + ... | Test.java:76:34:76:45 | clientSecret : String | Test.java:76:21:76:45 | ... + ... | This $@ is written to a log file. | Test.java:76:34:76:45 | clientSecret | potentially sensitive information | +| Test.java:77:21:77:42 | ... + ... | Test.java:77:34:77:42 | apiSecret : String | Test.java:77:21:77:42 | ... + ... | This $@ is written to a log file. | Test.java:77:34:77:42 | apiSecret | potentially sensitive information | +| Test.java:78:21:78:44 | ... + ... | Test.java:78:33:78:44 | sessionToken : String | Test.java:78:21:78:44 | ... + ... | This $@ is written to a log file. | Test.java:78:33:78:44 | sessionToken | potentially sensitive information | +| Test.java:79:21:79:43 | ... + ... | Test.java:79:33:79:43 | bearerToken : String | Test.java:79:21:79:43 | ... + ... | This $@ is written to a log file. | Test.java:79:33:79:43 | bearerToken | potentially sensitive information | +| Test.java:80:21:80:39 | ... + ... | Test.java:80:31:80:39 | secretKey : String | Test.java:80:21:80:39 | ... + ... | This $@ is written to a log file. | Test.java:80:31:80:39 | secretKey | potentially sensitive information | +| Test.java:81:21:81:44 | ... + ... | Test.java:81:33:81:44 | refreshToken : String | Test.java:81:21:81:44 | ... + ... | This $@ is written to a log file. | Test.java:81:33:81:44 | refreshToken | potentially sensitive information | +| Test.java:82:21:82:43 | ... + ... | Test.java:82:33:82:43 | secretValue : String | Test.java:82:21:82:43 | ... + ... | This $@ is written to a log file. | Test.java:82:33:82:43 | secretValue | potentially sensitive information | edges | Test.java:11:46:11:53 | password : String | Test.java:11:21:11:53 | ... + ... | provenance | Sink:MaD:2 | | Test.java:12:44:12:52 | authToken : String | Test.java:12:22:12:52 | ... + ... | provenance | Sink:MaD:1 | @@ -18,14 +19,15 @@ edges | Test.java:21:44:21:67 | substring(...) : String | Test.java:21:22:21:75 | ... + ... | provenance | Sink:MaD:1 | | Test.java:22:44:22:52 | authToken : String | Test.java:22:44:22:67 | substring(...) : String | provenance | MaD:3 | | Test.java:22:44:22:67 | substring(...) : String | Test.java:22:22:22:75 | ... + ... | provenance | Sink:MaD:1 | -| Test.java:66:33:66:43 | accessToken : String | Test.java:66:21:66:43 | ... + ... | provenance | Sink:MaD:2 | -| Test.java:67:34:67:45 | clientSecret : String | Test.java:67:21:67:45 | ... + ... | provenance | Sink:MaD:2 | -| Test.java:68:34:68:42 | apiSecret : String | Test.java:68:21:68:42 | ... + ... | provenance | Sink:MaD:2 | -| Test.java:69:33:69:44 | sessionToken : String | Test.java:69:21:69:44 | ... + ... | provenance | Sink:MaD:2 | -| Test.java:70:33:70:43 | bearerToken : String | Test.java:70:21:70:43 | ... + ... | provenance | Sink:MaD:2 | -| Test.java:71:31:71:39 | secretKey : String | Test.java:71:21:71:39 | ... + ... | provenance | Sink:MaD:2 | -| Test.java:72:33:72:44 | refreshToken : String | Test.java:72:21:72:44 | ... + ... | provenance | Sink:MaD:2 | -| Test.java:73:33:73:43 | secretValue : String | Test.java:73:21:73:43 | ... + ... | provenance | Sink:MaD:2 | +| Test.java:31:30:31:37 | password : String | Test.java:31:21:31:37 | ... + ... | provenance | Sink:MaD:2 | +| Test.java:75:33:75:43 | accessToken : String | Test.java:75:21:75:43 | ... + ... | provenance | Sink:MaD:2 | +| Test.java:76:34:76:45 | clientSecret : String | Test.java:76:21:76:45 | ... + ... | provenance | Sink:MaD:2 | +| Test.java:77:34:77:42 | apiSecret : String | Test.java:77:21:77:42 | ... + ... | provenance | Sink:MaD:2 | +| Test.java:78:33:78:44 | sessionToken : String | Test.java:78:21:78:44 | ... + ... | provenance | Sink:MaD:2 | +| Test.java:79:33:79:43 | bearerToken : String | Test.java:79:21:79:43 | ... + ... | provenance | Sink:MaD:2 | +| Test.java:80:31:80:39 | secretKey : String | Test.java:80:21:80:39 | ... + ... | provenance | Sink:MaD:2 | +| Test.java:81:33:81:44 | refreshToken : String | Test.java:81:21:81:44 | ... + ... | provenance | Sink:MaD:2 | +| Test.java:82:33:82:43 | secretValue : String | Test.java:82:21:82:43 | ... + ... | provenance | Sink:MaD:2 | models | 1 | Sink: org.apache.logging.log4j; Logger; true; error; (String); ; Argument[0]; log-injection; manual | | 2 | Sink: org.apache.logging.log4j; Logger; true; info; (String); ; Argument[0]; log-injection; manual | @@ -41,20 +43,22 @@ nodes | Test.java:22:22:22:75 | ... + ... | semmle.label | ... + ... | | Test.java:22:44:22:52 | authToken : String | semmle.label | authToken : String | | Test.java:22:44:22:67 | substring(...) : String | semmle.label | substring(...) : String | -| Test.java:66:21:66:43 | ... + ... | semmle.label | ... + ... | -| Test.java:66:33:66:43 | accessToken : String | semmle.label | accessToken : String | -| Test.java:67:21:67:45 | ... + ... | semmle.label | ... + ... | -| Test.java:67:34:67:45 | clientSecret : String | semmle.label | clientSecret : String | -| Test.java:68:21:68:42 | ... + ... | semmle.label | ... + ... | -| Test.java:68:34:68:42 | apiSecret : String | semmle.label | apiSecret : String | -| Test.java:69:21:69:44 | ... + ... | semmle.label | ... + ... | -| Test.java:69:33:69:44 | sessionToken : String | semmle.label | sessionToken : String | -| Test.java:70:21:70:43 | ... + ... | semmle.label | ... + ... | -| Test.java:70:33:70:43 | bearerToken : String | semmle.label | bearerToken : String | -| Test.java:71:21:71:39 | ... + ... | semmle.label | ... + ... | -| Test.java:71:31:71:39 | secretKey : String | semmle.label | secretKey : String | -| Test.java:72:21:72:44 | ... + ... | semmle.label | ... + ... | -| Test.java:72:33:72:44 | refreshToken : String | semmle.label | refreshToken : String | -| Test.java:73:21:73:43 | ... + ... | semmle.label | ... + ... | -| Test.java:73:33:73:43 | secretValue : String | semmle.label | secretValue : String | +| Test.java:31:21:31:37 | ... + ... | semmle.label | ... + ... | +| Test.java:31:30:31:37 | password : String | semmle.label | password : String | +| Test.java:75:21:75:43 | ... + ... | semmle.label | ... + ... | +| Test.java:75:33:75:43 | accessToken : String | semmle.label | accessToken : String | +| Test.java:76:21:76:45 | ... + ... | semmle.label | ... + ... | +| Test.java:76:34:76:45 | clientSecret : String | semmle.label | clientSecret : String | +| Test.java:77:21:77:42 | ... + ... | semmle.label | ... + ... | +| Test.java:77:34:77:42 | apiSecret : String | semmle.label | apiSecret : String | +| Test.java:78:21:78:44 | ... + ... | semmle.label | ... + ... | +| Test.java:78:33:78:44 | sessionToken : String | semmle.label | sessionToken : String | +| Test.java:79:21:79:43 | ... + ... | semmle.label | ... + ... | +| Test.java:79:33:79:43 | bearerToken : String | semmle.label | bearerToken : String | +| Test.java:80:21:80:39 | ... + ... | semmle.label | ... + ... | +| Test.java:80:31:80:39 | secretKey : String | semmle.label | secretKey : String | +| Test.java:81:21:81:44 | ... + ... | semmle.label | ... + ... | +| Test.java:81:33:81:44 | refreshToken : String | semmle.label | refreshToken : String | +| Test.java:82:21:82:43 | ... + ... | semmle.label | ... + ... | +| Test.java:82:33:82:43 | secretValue : String | semmle.label | secretValue : String | subpaths diff --git a/java/ql/test/query-tests/security/CWE-532/Test.java b/java/ql/test/query-tests/security/CWE-532/Test.java index 5c9826ba2cc..893d8a608d1 100644 --- a/java/ql/test/query-tests/security/CWE-532/Test.java +++ b/java/ql/test/query-tests/security/CWE-532/Test.java @@ -22,6 +22,15 @@ class Test { logger.error("Auth failed for: " + authToken.substring(0,8) + "..."); // $ Alert } + // Tests for hash/encryption sanitizer + void testHashSanitizer(String password, String authToken) { + Logger logger = null; + logger.info("hash: " + hashPassword(password)); // Safe - hashed + logger.info("hash: " + sha256Digest(authToken)); // Safe - digested + logger.info("enc: " + encryptValue(password)); // Safe - encrypted + logger.info("pw: " + password); // $ Alert // not hashed + } + // Tests for false positive exclusions: variables with "token" or "secret" in the name // that do not hold sensitive data. void testFalsePositiveExclusions( @@ -72,4 +81,8 @@ class Test { logger.info("token: " + refreshToken); // $ Alert logger.info("value: " + secretValue); // $ Alert } + + static String hashPassword(String input) { return input; } + static String sha256Digest(String input) { return input; } + static String encryptValue(String input) { return input; } }