enable .NET analyzer, fix security warnings (#1175)

- fix security warnings and some low hanging fruit refactoring
Azure · May 14, 2021 · c7ee673 · c7ee673
1 parent a85b368
commit c7ee673
Show file tree

Hide file tree

Showing 32 changed files with 304 additions and 266 deletions.
diff --git a/.editorconfig b/.editorconfig
diff --git a/common.props b/common.props
@@ -13,6 +13,7 @@
     <PackageRequireLicenseAcceptance>true</PackageRequireLicenseAcceptance>
     <PackageTags>Industrial;Industrial IoT;Manufacturing;Azure;IoT;.NET</PackageTags>
     <HighEntropyVA>true</HighEntropyVA>
+    <EnableNETAnalyzers>true</EnableNETAnalyzers>
     <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
     <NeutralLanguage>en-US</NeutralLanguage>
     <LangVersion>8.0</LangVersion>
@@ -40,6 +41,12 @@
   <PropertyGroup Condition="'$(FX_COP)' != ''">
     <TreatWarningsAsErrors>false</TreatWarningsAsErrors>
   </PropertyGroup>
+  <ItemGroup Condition="'$(SDL)' == 'true'">
+    <PackageReference Include="Roslynator.Analyzers" Version="3.1.0" PrivateAssets="All" />
+  </ItemGroup>
+  <PropertyGroup Condition="'$(SDL)' == 'true'">
+    <TreatWarningsAsErrors>false</TreatWarningsAsErrors>
+  </PropertyGroup>
   <PropertyGroup>
     <PrereleaseSuffix>$(PrereleaseVersionNoLeadingHyphen)-$(GitCommitIdShort)</PrereleaseSuffix>
     <PrereleaseVersion Condition="'$(BUILD_SOURCEVERSION)' != ''">[$(NuGetPackageVersion)]</PrereleaseVersion>

diff --git a/common/src/Microsoft.Azure.IIoT.Core/src/Extensions/ByteArrayEx.cs b/common/src/Microsoft.Azure.IIoT.Core/src/Extensions/ByteArrayEx.cs
@@ -69,11 +69,14 @@ public static class ByteArrayEx {
             return Convert.ToBase64String(value);
         }
 
+
         /// <summary>
         /// Hashes the string
         /// </summary>
         /// <param name="bytestr">string to hash</param>
         /// <returns></returns>
+        [Diagnostics.CodeAnalysis.SuppressMessage("Security", "CA5350:Do Not Use Weak Cryptographic Algorithms", 
+            Justification = "SHA1 not used for crypto operation.")]
         public static string ToSha1Hash(this byte[] bytestr) {
             if (bytestr == null) {
                 return null;

diff --git a/common/src/Microsoft.Azure.IIoT.Core/src/Extensions/StreamEx.cs b/common/src/Microsoft.Azure.IIoT.Core/src/Extensions/StreamEx.cs
@@ -8,6 +8,7 @@ namespace System.IO {
     using System.Text;
     using System.Threading;
     using System.Threading.Tasks;
+    using System.Xml;
 
     /// <summary>
     /// Stream extensions
@@ -51,7 +52,11 @@ public static class StreamEx {
             var reader = new StreamReader(stream);
             try {
                 var serializer = new Xml.Serialization.XmlSerializer(typeof(T));
-                return (T)serializer.Deserialize(reader);
+                var xmlReader = new XmlTextReader(reader) {
+                    DtdProcessing = DtdProcessing.Prohibit,
+                    XmlResolver = null
+                };
+                return (T)serializer.Deserialize(xmlReader);
             }
             finally {
                 reader.Close();

diff --git a/common/src/Microsoft.Azure.IIoT.Core/src/Messaging/Default/EventBusHost.cs b/common/src/Microsoft.Azure.IIoT.Core/src/Messaging/Default/EventBusHost.cs
@@ -58,7 +58,7 @@ public class EventBusHost : IHostProcess {
                         catch (Exception ex) {
                             _logger.Error(ex, "Failed to start Event bus host for {type}.",
                                 type.Name);
-                            throw ex;
+                            throw;
                         }
                     }
                 }
@@ -82,7 +82,7 @@ public class EventBusHost : IHostProcess {
                     catch (Exception ex) {
                         _logger.Error(ex, "Failed to stop Event bus host using token {token}.",
                             token);
-                        throw ex;
+                        throw;
                     }
                 }
                 _handlers.Clear();

diff --git a/common/src/Microsoft.Azure.IIoT.Core/src/Utils/HostAutoStart.cs b/common/src/Microsoft.Azure.IIoT.Core/src/Utils/HostAutoStart.cs
@@ -35,7 +35,7 @@ public class HostAutoStart : IDisposable, IStartable {
             }
             catch (Exception ex) {
                 _logger.Error(ex, "Failed to start some hosts.");
-                throw ex;
+                throw;
             }
         }
 

diff --git a/common/src/Microsoft.Azure.IIoT.Http.SignalR/src/Services/SignalRHubClientHost.cs b/common/src/Microsoft.Azure.IIoT.Http.SignalR/src/Services/SignalRHubClientHost.cs
@@ -81,7 +81,7 @@ public class SignalRHubClientHost : ICallbackRegistrar, IHostProcess {
             catch (Exception ex) {
                 _started = false;
                 _logger.Error(ex, "Error starting SignalR client host.");
-                throw ex;
+                throw;
             }
             finally {
                 _lock.Release();

diff --git a/common/src/Microsoft.Azure.IIoT.Hub.Module.Client/src/Default/IoTSdkFactory.cs b/common/src/Microsoft.Azure.IIoT.Hub.Module.Client/src/Default/IoTSdkFactory.cs
@@ -140,7 +140,10 @@ public sealed class IoTSdkFactory : IClientFactory, IDisposable {
             _logHook?.Dispose();
         }
 
+
         /// <inheritdoc/>
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Security", "CA5359:Do Not Disable Certificate Validation", 
+            Justification = "<Pending>")]
         public async Task<IClient> CreateAsync(string product, IProcessControl ctrl) {
 
             if (_bypassCertValidation) {

diff --git a/common/src/Microsoft.Azure.IIoT.Hub.Module.Framework/src/Hosting/ModuleHost.cs b/common/src/Microsoft.Azure.IIoT.Hub.Module.Framework/src/Hosting/ModuleHost.cs
@@ -92,7 +92,7 @@ public sealed class ModuleHost : IModuleHost, ITwinProperties, IEventEmitter,
                 }
                 finally {
                     kModuleStart.WithLabels(DeviceId ?? "", ModuleId ?? "", _moduleGuid, "",
-                        DateTime.UtcNow.ToString("yyyy-MM-dd'T'HH:mm:ss.FFFFFFFK", 
+                        DateTime.UtcNow.ToString("yyyy-MM-dd'T'HH:mm:ss.FFFFFFFK",
                         CultureInfo.InvariantCulture)).Set(0);
                     Client?.Dispose();
                     Client = null;
@@ -153,7 +153,7 @@ public sealed class ModuleHost : IModuleHost, ITwinProperties, IEventEmitter,
                         return;
                     }
                 }
-                catch (Exception ex) {
+                catch (Exception) {
                     kModuleStart.WithLabels(DeviceId ?? "", ModuleId ?? "",
                         _moduleGuid, version,
                         DateTime.UtcNow.ToString("yyyy-MM-dd'T'HH:mm:ss.FFFFFFFK",
@@ -166,7 +166,7 @@ public sealed class ModuleHost : IModuleHost, ITwinProperties, IEventEmitter,
                     ModuleId = null;
                     SiteId = null;
                     Gateway = null;
-                    throw ex;
+                    throw;
                 }
                 finally {
                     _lock.Release();
@@ -544,7 +544,7 @@ public sealed class ModuleHost : IModuleHost, ITwinProperties, IEventEmitter,
         private static readonly Gauge kModuleStart = Metrics
             .CreateGauge("iiot_edge_module_start", "starting module",
                 new GaugeConfiguration {
-                    LabelNames = new[] {"deviceid", "module", "runid", "version", "timestamp_utc" }
+                    LabelNames = new[] { "deviceid", "module", "runid", "version", "timestamp_utc" }
                 });
     }
 }
diff --git a/common/src/Microsoft.Azure.IIoT.Hub.Module.Framework/src/Hosting/SettingsRouter.cs b/common/src/Microsoft.Azure.IIoT.Hub.Module.Framework/src/Hosting/SettingsRouter.cs
@@ -444,7 +444,7 @@ private class PropertyInvoker {
                     _logger.Warning(e,
                         "Exception during setter {controller} {name} invocation",
                         _controller.Target.GetType().Name, _property.Name);
-                    throw e;
+                    throw;
                 }
             }
 
@@ -475,7 +475,7 @@ private class PropertyInvoker {
                     _logger.Warning(e,
                         "Exception during getter {controller} {name} invocation",
                         _controller.Target.GetType().Name, _property.Name);
-                    throw e;
+                    throw;
                 }
             }
 
@@ -508,7 +508,7 @@ private class PropertyInvoker {
                     _logger.Warning(e,
                         "Exception collecting all indexed values on {controller}.",
                         _controller.Target.GetType().Name);
-                    throw e;
+                    throw;
                 }
             }
 

diff --git a/common/src/Microsoft.Azure.IIoT.Hub.Processor/src/EventHub/EventProcessorHost.cs b/common/src/Microsoft.Azure.IIoT.Hub.Processor/src/EventHub/EventProcessorHost.cs
@@ -103,7 +103,7 @@ public sealed class EventProcessorHost : IDisposable, IEventProcessingHost, IHos
             catch (Exception ex) {
                 _logger.Error(ex, "Error starting event processor host.");
                 _host = null;
-                throw ex;
+                throw;
             }
             finally {
                 _lock.Release();

diff --git a/common/src/Microsoft.Azure.IIoT.Messaging.ServiceBus/src/Clients/ServiceBusClientFactory.cs b/common/src/Microsoft.Azure.IIoT.Messaging.ServiceBus/src/Clients/ServiceBusClientFactory.cs
@@ -141,7 +141,7 @@ public class ServiceBusClientFactory : IServiceBusClientFactory {
                         continue;
                     }
                     _logger.Error(ex, "Failed to create subscription client.");
-                    throw ex;
+                    throw;
                 }
             }
         }
@@ -172,7 +172,7 @@ public class ServiceBusClientFactory : IServiceBusClientFactory {
                         continue; // 429
                     }
                     _logger.Error(ex, "Failed to create queue client.");
-                    throw ex;
+                    throw;
                 }
             }
         }
@@ -202,7 +202,7 @@ public class ServiceBusClientFactory : IServiceBusClientFactory {
                         continue; // 429
                     }
                     _logger.Error(ex, "Failed to create topic client.");
-                    throw ex;
+                    throw;
                 }
             }
         }

diff --git a/common/src/Microsoft.Azure.IIoT.Messaging.ServiceBus/src/Services/ServiceBusEventBus.cs b/common/src/Microsoft.Azure.IIoT.Messaging.ServiceBus/src/Services/ServiceBusEventBus.cs
@@ -112,7 +112,7 @@ public class ServiceBusEventBus : IEventBus {
                                 eventName);
                         }
                         else {
-                            throw ex;
+                            throw;
                         }
                     }
                     handlers = new Dictionary<string, Subscription>();

diff --git a/common/src/Microsoft.Azure.IIoT.Messaging.SignalR/src/Services/SignalRServiceHost.cs b/common/src/Microsoft.Azure.IIoT.Messaging.SignalR/src/Services/SignalRServiceHost.cs
@@ -84,7 +84,7 @@ public SignalRServiceHost(ISignalRServiceConfig config, ILogger logger)
             }
             catch (Exception ex) {
                 _logger.Error(ex, "Failed to start SignalR service host.");
-                throw ex;
+                throw;
             }
         }
 

diff --git a/components/opc-ua/src/Microsoft.Azure.IIoT.OpcUa.Cdm/src/Storage/CdmFileStorageAdapter.cs b/components/opc-ua/src/Microsoft.Azure.IIoT.OpcUa.Cdm/src/Storage/CdmFileStorageAdapter.cs
@@ -70,7 +70,7 @@ public class CdmFileStorageAdapter : NetworkAdapter, IStorageAdapter, IDisposabl
             }
             catch (Exception ex) {
                 _logger.Error(ex, "Failed to read data from {corpus}", corpusPath);
-                throw ex;
+                throw;
             }
         }
 
@@ -89,7 +89,7 @@ public class CdmFileStorageAdapter : NetworkAdapter, IStorageAdapter, IDisposabl
             }
             catch (Exception ex) {
                 _logger.Error(ex, "Failed to write data to {corpus}", corpusPath);
-                throw ex;
+                throw;
             }
         }
 
@@ -111,7 +111,7 @@ public class CdmFileStorageAdapter : NetworkAdapter, IStorageAdapter, IDisposabl
             }
             catch (Exception ex) {
                 _logger.Error(ex, "Failed to write data to {corpus}", corpusPath);
-                throw ex;
+                throw;
             }
         }
 
@@ -124,7 +124,7 @@ public class CdmFileStorageAdapter : NetworkAdapter, IStorageAdapter, IDisposabl
             }
             catch (Exception ex) {
                 _logger.Error(ex, "Failed to get files in {corpus}", folderCorpusPath);
-                throw ex;
+                throw;
             }
         }
 

diff --git a/...nents/opc-ua/src/Microsoft.Azure.IIoT.OpcUa.Protocol/src/Extensions/EndpointServicesEx.cs b/...nents/opc-ua/src/Microsoft.Azure.IIoT.OpcUa.Protocol/src/Extensions/EndpointServicesEx.cs
@@ -20,8 +20,8 @@ public static class EndpointServicesEx {
         /// </summary>
         /// <typeparam name="T"></typeparam>
         /// <param name="client"></param>
-        /// <param name="ct"></param>
         /// <param name="connection"></param>
+        /// <param name="ct"></param>
         /// <param name="service"></param>
         /// <returns></returns>
         public static Task<T> ExecuteServiceAsync<T>(this IEndpointServices client,
@@ -34,8 +34,8 @@ public static class EndpointServicesEx {
         /// </summary>
         /// <typeparam name="T"></typeparam>
         /// <param name="client"></param>
-        /// <param name="ct"></param>
         /// <param name="connection"></param>
+        /// <param name="ct"></param>
         /// <param name="service"></param>
         /// <param name="handler"></param>
         /// <returns></returns>
@@ -73,8 +73,8 @@ public static class EndpointServicesEx {
         /// </summary>
         /// <typeparam name="T"></typeparam>
         /// <param name="client"></param>
-        /// <param name="elevation"></param>
         /// <param name="endpoint"></param>
+        /// <param name="elevation"></param>
         /// <param name="timeout"></param>
         /// <param name="service"></param>
         /// <returns></returns>
@@ -107,8 +107,8 @@ public static class EndpointServicesEx {
         /// <typeparam name="T"></typeparam>
         /// <param name="client"></param>
         /// <param name="elevation"></param>
-        /// <param name="priority"></param>
         /// <param name="endpoint"></param>
+        /// <param name="priority"></param>
         /// <param name="ct"></param>
         /// <param name="service"></param>
         /// <returns></returns>
@@ -125,8 +125,8 @@ public static class EndpointServicesEx {
         /// <typeparam name="T"></typeparam>
         /// <param name="client"></param>
         /// <param name="elevation"></param>
-        /// <param name="priority"></param>
         /// <param name="endpoint"></param>
+        /// <param name="priority"></param>
         /// <param name="ct"></param>
         /// <param name="service"></param>
         /// <param name="handler"></param>

diff --git a/components/opc-ua/src/Microsoft.Azure.IIoT.OpcUa.Protocol/src/Extensions/FilterEncoderEx.cs b/components/opc-ua/src/Microsoft.Azure.IIoT.OpcUa.Protocol/src/Extensions/FilterEncoderEx.cs
@@ -36,8 +36,8 @@ public static class FilterEncoderEx {
         /// <summary>
         /// Convert to stack model
         /// </summary>
-        /// <param name="model"></param>
         /// <param name="encoder"></param>
+        /// <param name="model"></param>
         /// <param name="noDefaultFilter"></param>
         /// <returns></returns>
         public static EventFilter Decode(this IVariantEncoder encoder, EventFilterModel model,
@@ -60,8 +60,8 @@ public static class FilterEncoderEx {
         /// <summary>
         /// Convert to stack model
         /// </summary>
-        /// <param name="model"></param>
         /// <param name="encoder"></param>
+        /// <param name="model"></param>
         /// <returns></returns>
         public static EventFilterModel Encode(this IVariantEncoder encoder, EventFilter model) {
             if (model == null) {
@@ -78,8 +78,8 @@ public static class FilterEncoderEx {
         /// <summary>
         /// Convert to stack model
         /// </summary>
-        /// <param name="model"></param>
         /// <param name="encoder"></param>
+        /// <param name="model"></param>
         /// <param name="onlySimpleAttributeOperands"></param>
         /// <returns></returns>
         public static ContentFilter Decode(this IVariantEncoder encoder, ContentFilterModel model,
@@ -97,8 +97,8 @@ public static class FilterEncoderEx {
         /// <summary>
         /// Convert to service model
         /// </summary>
-        /// <param name="model"></param>
         /// <param name="encoder"></param>
+        /// <param name="model"></param>
         /// <returns></returns>
         public static ContentFilterModel Encode(this IVariantEncoder encoder, ContentFilter model) {
             if (model == null) {
@@ -114,8 +114,8 @@ public static class FilterEncoderEx {
         /// <summary>
         /// Convert to stack model
         /// </summary>
-        /// <param name="model"></param>
         /// <param name="encoder"></param>
+        /// <param name="model"></param>
         /// <param name="onlySimpleAttributeOperands"></param>
         /// <returns></returns>
         public static ContentFilterElement Decode(this IVariantEncoder encoder,
@@ -135,8 +135,8 @@ public static class FilterEncoderEx {
         /// <summary>
         /// Convert to service model
         /// </summary>
-        /// <param name="model"></param>
         /// <param name="encoder"></param>
+        /// <param name="model"></param>
         /// <returns></returns>
         public static ContentFilterElementModel Encode(this IVariantEncoder encoder,
             ContentFilterElement model) {
@@ -156,8 +156,8 @@ public static class FilterEncoderEx {
         /// <summary>
         /// Convert to stack model
         /// </summary>
-        /// <param name="model"></param>
         /// <param name="encoder"></param>
+        /// <param name="model"></param>
         /// <param name="onlySimpleAttributeOperands"></param>
         /// <returns></returns>
         public static FilterOperand Decode(this IVariantEncoder encoder,
@@ -197,8 +197,8 @@ public static class FilterEncoderEx {
         /// <summary>
         /// Convert to service model
         /// </summary>
-        /// <param name="model"></param>
         /// <param name="encoder"></param>
+        /// <param name="model"></param>
         /// <returns></returns>
         public static FilterOperandModel Encode(this IVariantEncoder encoder,
             FilterOperand model) {

diff --git a/...onents/opc-ua/src/Microsoft.Azure.IIoT.OpcUa.Protocol/src/Extensions/OperationResultEx.cs b/...onents/opc-ua/src/Microsoft.Azure.IIoT.OpcUa.Protocol/src/Extensions/OperationResultEx.cs
@@ -34,8 +34,8 @@ public static class OperationResultEx {
         /// <typeparam name="T"></typeparam>
         /// <param name="operation"></param>
         /// <param name="results"></param>
-        /// <param name="diagnostics"></param>
         /// <param name="operations"></param>
+        /// <param name="diagnostics"></param>
         /// <param name="requested"></param>
         /// <param name="traceOnly"></param>
         public static void Validate<T>(string operation,