From 02c035474428ade722c07d2e2a54d1c6b1fce646 Mon Sep 17 00:00:00 2001
From: dmiller <dmiller@e0a8ed71-7df4-0310-8962-fdc924857419>
Date: Wed, 15 Dec 2021 23:44:41 +0000
Subject: [PATCH] Avoid TOCTOU by checking discovery state in mssql.Discover

A script might check WasDiscoveryPerformed and get a negative response,
then call Discover, during which time another script already called
Discover. Instead, check the condition *after* acquiring the mutex.
---
 nselib/mssql.lua        | 18 ++++++++++--------
 scripts/ms-sql-dac.nse  |  4 +---
 scripts/ms-sql-info.nse |  4 +---
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/nselib/mssql.lua b/nselib/mssql.lua
index fe5946b1d..ce8f48176 100644
--- a/nselib/mssql.lua
+++ b/nselib/mssql.lua
@@ -2731,12 +2731,16 @@ Helper =
   --
   --  @param host Host table as received by the script action function
   Discover = function( host )
-    nmap.registry.mssql = nmap.registry.mssql or {}
-    nmap.registry.mssql.discovery_performed = nmap.registry.mssql.discovery_performed or {}
-    nmap.registry.mssql.discovery_performed[ host.ip ] = false
-
     local mutex = nmap.mutex( "discovery_performed for " .. host.ip )
     mutex( "lock" )
+    nmap.registry.mssql = nmap.registry.mssql or {}
+    nmap.registry.mssql.discovery_performed = nmap.registry.mssql.discovery_performed or {}
+    if nmap.registry.mssql.discovery_performed[ host.ip ] then
+      mutex "done"
+      return
+    end
+    nmap.registry.mssql.discovery_performed[ host.ip ] = false
+
 
     local sqlDefaultPort = nmap.get_port_state( host, {number = 1433, protocol = "tcp"} ) or {number = 1433, protocol = "tcp"}
     local sqlBrowserPort = nmap.get_port_state( host, {number = 1434, protocol = "udp"} ) or {number = 1434, protocol = "udp"}
@@ -3150,10 +3154,8 @@ Helper =
         return false, "No instance(s) specified."
       end
 
-      if ( not Helper.WasDiscoveryPerformed( host ) ) then
-        stdnse.debug2("%s: Discovery has not been performed prior to GetTargetInstances() call. Performing discovery now.", "MSSQL" )
-        Helper.Discover( host )
-      end
+      -- Perform discovery. This won't do anything if it's already been done.
+      Helper.Discover( host )
 
       local instanceList = Helper.GetDiscoveredInstances( host )
       if ( not instanceList ) then
diff --git a/scripts/ms-sql-dac.nse b/scripts/ms-sql-dac.nse
index 6cc67d9fe..121e7242b 100644
--- a/scripts/ms-sql-dac.nse
+++ b/scripts/ms-sql-dac.nse
@@ -76,9 +76,7 @@ action = function( host )
   local status, instanceList = mssql.Helper.GetTargetInstances( host )
   -- if no instances were targeted, then display info on all
   if ( not status ) then
-    if ( not mssql.Helper.WasDiscoveryPerformed( host ) ) then
-      mssql.Helper.Discover( host )
-    end
+    mssql.Helper.Discover( host )
     instanceList = mssql.Helper.GetDiscoveredInstances( host )
   end
 
diff --git a/scripts/ms-sql-info.nse b/scripts/ms-sql-info.nse
index dee176a75..df76b6737 100644
--- a/scripts/ms-sql-info.nse
+++ b/scripts/ms-sql-info.nse
@@ -254,9 +254,7 @@ action = function( host )
   local status, instanceList = mssql.Helper.GetTargetInstances( host )
   -- if no instances were targeted, then display info on all
   if ( not status ) then
-    if ( not mssql.Helper.WasDiscoveryPerformed( host ) ) then
-      mssql.Helper.Discover( host )
-    end
+    mssql.Helper.Discover( host )
     instanceList = mssql.Helper.GetDiscoveredInstances( host )
   end