none

Author: BrennanConroy

src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.cpp

@@ -12,6 +12,9 @@ HRESULT AppOfflineApplication::CreateHandler(IHttpContext* pHttpContext, IREQUES
     try
     {
         auto handler = std::make_unique<AppOfflineHandler>(*pHttpContext, m_strAppOfflineContent);
+        // 'Don't return a pointer that may be invalid'
+        // We use make_unique to prevent memory leaks in case of exception during ctor and then release it so we don't delete the pointer
+#pragma warning(suppress: 26487)
         *pRequestHandler = handler.release();
     }
     CATCH_RETURN();

src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/ModuleEnvironment.cpp

@@ -9,8 +9,8 @@
 extern DWORD g_dwIISServerVersion;
 
 static std::wstring GetIISVersion() {
-    int major = (int)(g_dwIISServerVersion >> 16);
-    int minor = (int)(g_dwIISServerVersion & 0xffff);
+    const int major = (int)(g_dwIISServerVersion >> 16);
+    const int minor = (int)(g_dwIISServerVersion & 0xffff);
 
     std::wstringstream version;
     version << major << "." << minor;
@@ -44,14 +44,17 @@ void SetApplicationEnvironmentVariables(_In_ IHttpServer &server, _In_ IHttpCont
 
     IHttpServer2* server2;
     if (SUCCEEDED(HttpGetExtendedInterface(&server, &server, &server2))) {
+        // GetAppPoolConfigFile likely returns a 0 terminating string but the SAL annotations don't state that, we should probably change the code
+        // just to be safe
+#pragma warning(suppress: 6387)
         SetEnvironmentVariable(L"ASPNETCORE_IIS_APP_POOL_CONFIG_FILE", server2->GetAppPoolConfigFile());
     }
 
-    IHttpSite* site = pHttpContext.GetSite();
+    const IHttpSite* site = pHttpContext.GetSite();
     SetEnvironmentVariable(L"ASPNETCORE_IIS_SITE_NAME", site->GetSiteName());
     SetEnvironmentVariable(L"ASPNETCORE_IIS_SITE_ID", std::to_wstring(site->GetSiteId()).c_str());
 
-    IHttpApplication* app = pHttpContext.GetApplication();
+    const IHttpApplication* app = pHttpContext.GetApplication();
     SetEnvironmentVariable(L"ASPNETCORE_IIS_APP_CONFIG_PATH", app->GetAppConfigPath());
     SetEnvironmentVariable(L"ASPNETCORE_IIS_APPLICATION_ID", app->GetApplicationId());
     SetEnvironmentVariable(L"ASPNETCORE_IIS_APPLICATION_VIRTUAL_PATH", ToVirtualPath(app->GetAppConfigPath()).c_str());

src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/ShimOptions.cpp

@@ -79,9 +79,9 @@ ShimOptions::ShimOptions(const ConfigurationSource &configurationSource) :
     m_strArguments = Environment::GetEnvironmentVariableValue(CS_ANCM_LAUNCHER_ARGS)
         .value_or(m_strArguments);
 
-    auto detailedErrorsEnabled = equals_ignore_case(L"1", detailedErrors) || equals_ignore_case(L"true", detailedErrors);
-    auto aspnetCoreEnvironmentEnabled = equals_ignore_case(L"Development", aspnetCoreEnvironment);
-    auto dotnetEnvironmentEnabled = equals_ignore_case(L"Development", dotnetEnvironment);
+    const auto detailedErrorsEnabled = equals_ignore_case(L"1", detailedErrors) || equals_ignore_case(L"true", detailedErrors);
+    const auto aspnetCoreEnvironmentEnabled = equals_ignore_case(L"Development", aspnetCoreEnvironment);
+    const auto dotnetEnvironmentEnabled = equals_ignore_case(L"Development", dotnetEnvironment);
 
     m_fShowDetailedErrors = detailedErrorsEnabled || aspnetCoreEnvironmentEnabled || dotnetEnvironmentEnabled;
 
@@ -111,7 +111,7 @@ ShimOptions::ShimOptions(const ConfigurationSource &configurationSource) :
 
 void ShimOptions::SetShutdownDelay(const std::wstring& shutdownDelay)
 {
-    auto millsecondsValue = std::stoi(shutdownDelay);
+    const auto millsecondsValue = std::stoi(shutdownDelay);
     if (millsecondsValue < 0)
     {
         throw ConfigurationLoadException(format(

src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp

@@ -116,7 +116,7 @@ APPLICATION_INFO::CreateApplication(IHttpContext& pHttpContext)
                 pHttpApplication.GetApplicationId(),
                 hr);
 
-            auto page = ANCM_ERROR_PAGE;
+            constexpr auto page = ANCM_ERROR_PAGE;
             std::string responseContent;
             if (options.QueryShowDetailedErrors())
             {
@@ -301,7 +301,7 @@ APPLICATION_INFO::HandleShadowCopy(const ShimOptions& options, IHttpContext& pHt
         auto shadowCopyBaseDirectory = std::filesystem::directory_entry(shadowCopyPath);
         if (!shadowCopyBaseDirectory.exists())
         {
-            CreateDirectory(shadowCopyBaseDirectory.path().wstring().c_str(), NULL);
+            CreateDirectory(shadowCopyBaseDirectory.path().wstring().c_str(), nullptr);
         }
 
         for (auto& entry : std::filesystem::directory_iterator(shadowCopyPath))
@@ -311,7 +311,7 @@ APPLICATION_INFO::HandleShadowCopy(const ShimOptions& options, IHttpContext& pHt
                 try
                 {
                     auto tempDirName = entry.path().filename().string();
-                    int intFileName = std::stoi(tempDirName);
+                    const int intFileName = std::stoi(tempDirName);
                     if (intFileName > directoryName)
                     {
                         directoryName = intFileName;
@@ -334,7 +334,7 @@ APPLICATION_INFO::HandleShadowCopy(const ShimOptions& options, IHttpContext& pHt
         // Avoid using canonical for shadowCopyBaseDirectory
         // It could expand to a network drive, or an expanded link folder path
         // We already made it an absolute path relative to the physicalPath above
-        HRESULT hr = Environment::CopyToDirectory(physicalPath, shadowCopyPath, options.QueryCleanShadowCopyDirectory(), shadowCopyBaseDirectory.path(), copiedFileCount);
+        const HRESULT hr = Environment::CopyToDirectory(physicalPath, shadowCopyPath, options.QueryCleanShadowCopyDirectory(), shadowCopyBaseDirectory.path(), copiedFileCount);
 
         LOG_INFOF(L"Finished copying %d files to shadow copy directory %ls.", copiedFileCount, shadowCopyBaseDirectory.path().c_str());
 

src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/dllmain.cpp

@@ -57,6 +57,7 @@ BOOL WINAPI DllMain(HMODULE hModule,
         // this is a bug in IIS. To try to avoid AVs, we will set a global flag
         g_fInShutdown = TRUE;
         StaticCleanup();
+        break;
     default:
         break;
     }

src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.h

@@ -75,7 +75,7 @@ class ASPNET_CORE_GLOBAL_MODULE : NonCopyable, public CGlobalModule
             // IIS will actually stop giving us new requests and queue them instead for processing by the new app process.
             m_shutdown = std::thread([this]()
                 {
-                    auto delay = m_pApplicationManager->GetShutdownDelay();
+                    const auto delay = m_pApplicationManager->GetShutdownDelay();
                     LOG_INFOF(L"Shutdown starting in %d ms.", delay.count());
                     // Delay so that any incoming requests while we're returning from OnGlobalStopListening are allowed to be processed
                     std::this_thread::sleep_for(delay);

src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ErrorContext.h

@@ -5,6 +5,14 @@
 
 struct ErrorContext
 {
+    ErrorContext() :
+        detailedErrorContent(),
+        generalErrorType(),
+        errorReason(),
+        statusCode(),
+        subStatusCode()
+    {}
+
     // TODO consider adding HRESULT here
     std::string detailedErrorContent;
     USHORT statusCode;

src/Servers/IIS/AspNetCoreModuleV2/CommonLib/HostFxr.cpp

@@ -87,6 +87,9 @@ void HostFxr::Load(const std::wstring& location)
     }
 }
 
+// No array to pointer decay
+// This is in reference to main taking PCWSTR argv[] and the analyzer wanting us to use span instead
+#pragma warning(suppress: 26485)
 void HostFxr::SetMain(hostfxr_main_fn hostfxr_main_fn)
 {
     m_hostfxr_main_fn = hostfxr_main_fn;
@@ -114,7 +117,7 @@ HostFxrErrorRedirector HostFxr::RedirectOutput(RedirectionOutput* writer) const
     return HostFxrErrorRedirector(m_corehost_set_error_writer_fn, writer);
 }
 
-int HostFxr::InitializeForApp(int argc, PCWSTR* argv, const std::wstring& dotnetExe) const noexcept
+int HostFxr::InitializeForApp(int argc, PCWSTR* argv, const std::wstring& dotnetExe) const
 {
     if (m_hostfxr_initialize_for_dotnet_commandline_fn == nullptr || m_hostfxr_main_fn != nullptr)
     {

src/Servers/IIS/AspNetCoreModuleV2/CommonLib/HostFxr.h

@@ -30,7 +30,7 @@ typedef int(*hostfxr_get_runtime_property_value_fn)(void* host_context_handle, P
 typedef int(*hostfxr_run_app_fn)(void* host_context_handle);
 typedef int(*hostfxr_close_fn)(void* hostfxr_context_handle);
 
-const int AppArgNotRunnable = 0x80008094;
+constexpr int AppArgNotRunnable = 0x80008094;
 
 class HostFxrErrorRedirector: NonCopyable
 {
@@ -53,14 +53,22 @@ class HostFxr: NonCopyable
     {
     }
 
+    // No array to pointer decay
+    // This is in reference to main taking PCWSTR argv[] and the analyzer wanting us to use span instead
+#pragma warning(suppress: 26485)
     HostFxr(
         hostfxr_main_fn hostfxr_main_fn,
         hostfxr_get_native_search_directories_fn hostfxr_get_native_search_directories_fn,
         corehost_set_error_writer_fn corehost_set_error_writer_fn) noexcept
         : m_hostfxr_main_fn(hostfxr_main_fn),
           m_hostfxr_get_native_search_directories_fn(hostfxr_get_native_search_directories_fn),
           m_corehost_set_error_writer_fn(corehost_set_error_writer_fn),
-          m_host_context_handle(nullptr)
+          m_host_context_handle(nullptr),
+          m_hostfxr_close_fn(nullptr),
+          m_hostfxr_get_runtime_property_value_fn(nullptr),
+          m_hostfxr_initialize_for_dotnet_commandline_fn(nullptr),
+          m_hostfxr_run_app_fn(nullptr),
+          m_hostfxr_set_runtime_property_value_fn(nullptr)
     {
     }
 
@@ -78,7 +86,7 @@ class HostFxr: NonCopyable
     HostFxrErrorRedirector RedirectOutput(RedirectionOutput* writer) const noexcept;
     int SetRuntimePropertyValue(PCWSTR name, PCWSTR value) const noexcept;
     int GetRuntimePropertyValue(PCWSTR name, PWSTR* value) const noexcept;
-    int InitializeForApp(int argc, PCWSTR* argv, const std::wstring& m_dotnetExeKnownLocation) const noexcept;
+    int InitializeForApp(int argc, PCWSTR* argv, const std::wstring& m_dotnetExeKnownLocation) const;
     void Close() const noexcept;
 
 private:

src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ServerErrorApplication.h

@@ -24,6 +24,9 @@ class ServerErrorApplication : public PollingAppOfflineApplication
 
     HRESULT CreateHandler(IHttpContext *pHttpContext, IREQUEST_HANDLER ** pRequestHandler) override
     {
+        // 'Don't return a pointer that may be invalid'
+        // We use make_unique to prevent memory leaks and then release it so we don't delete the pointer
+#pragma warning(suppress: 26487)
         *pRequestHandler = std::make_unique<ServerErrorHandler>(*pHttpContext, m_statusCode, m_subStatusCode, m_statusText, m_HR, m_disableStartupPage, m_responseContent).release();
 
         return S_OK;