DefenderServices check (edge-cases)

[alan-null]

Didn't know if I should start a discussion or report it as an issue so let me know how do you want to handle code/logic related things in the future (issues or discussion first).

I am currently reading the whole code and found this

Sophia-Script-for-Windows/src/Sophia_Script_for_Windows_10/Module/Sophia.psm1

Lines 300 to 307 in 6692e83

	if ((Get-Service -Name $_).Status -eq "running")
	{
	$Script:DefenderServices = $true
	}
	else
	{
	$Script:DefenderServices = $false
	}

So there are 3 services that script checks via Foreach

• Windefend
• SecurityHealthService
• wscsvc

When we have following state:

Status   Name               DisplayName
------   ----               -----------
Running  Windefend          Microsoft Defender Antivirus Service
Running  SecurityHealthS... Windows Security Service
Running  wscsvc             Security Center

everything will work fine.

The potential problem I see here is when $Script:DefenderServices will be set to false by wscsvc. Potential edge-case (last service state other than Running)

Status   Name               DisplayName
------   ----               -----------
Running  Windefend          Microsoft Defender Antivirus Service
Running  SecurityHealthS... Windows Security Service
Stopped  wscsvc             Security Center

So basically Sophia uses only one flag where state of 3 different services is stored.

Haven't checked how this could actually influence the rest but it looks a bit illogical to me. The same false state will appear when all services are Stopped and only last one is Running

[alan-null]

@farag2 In case you missed. Please take a look 👆

[farag2]

I really forgot about this topic. Let take a look...

[farag2]

Well, I do not see any problem with the code. And it's not illogical ))

@("Windefend", "SecurityHealthService", "wscsvc") | ForEach-Object -Process {
	if ($null -eq (Get-Service -Name $_ -ErrorAction Ignore))
	{
		$Localization.WindowsBroken
		exit
	}
	else
	{
		if ((Get-Service -Name $_).Status -eq "running")
		{
			$Script:DefenderServices = $true
		}
		else
		{
			$Script:DefenderServices = $false
		}
	}
}

I check if all services exist. If not, we receive error and exit. Else we check if they're running. Correct me where the logic is broken? I've tested on many machines and VMs to isolate bugs, but logic works)

[alan-null]

@farag2
Here is an example code proving that the current implementation returns different results depending on order of services.
First if in the code is not an issue I am talking about else when all services exist

Clear-Host
# get example srvice with stopped status
$exampleStopped = Get-Service | ? { $_.Status -eq "stopped" } | Select-Object -First 1
# get example srvice with running status
$examplerunning = Get-Service | ? { $_.Status -eq "running" } | Select-Object -First 1

# Sophia code
function Test-Implementation {
    param ($services)

    $services | ForEach-Object -Process {
        if ($null -eq (Get-Service -Name $_ -ErrorAction Ignore)) {
            $Localization.WindowsBroken
            exit
        }
        else {
            if ((Get-Service -Name $_).Status -eq "running") {
                $Script:DefenderServices = $true
            }
            else {
                $Script:DefenderServices = $false
            }
        }
    }    
}

# Test scenarios

# services will be different, name doesn't matter but status for this case is STOPPED, RUNNING
$services = @($exampleStopped.Name, $examplerunning.Name)
Write-Host "Services: $($services -join ',')"
Test-Implementation $services
Write-Host "Script:DefenderServices = $($Script:DefenderServices)"
""

# services will be different, name doesn't matter but status for this case is RUNNING, STOPPED
$services = @($examplerunning.Name, $exampleStopped.Name)
Write-Host "Services: $($services -join ',')"
Test-Implementation $services
Write-Host "Script:DefenderServices = $($Script:DefenderServices)"

Result:

Services: aspnet_state,BDESVC
Script:DefenderServices = True

Services: BDESVC,aspnet_state
Script:DefenderServices = False

Do you get my point now?

[farag2]

Yeah, really, you've convinced me. How to fix?..

[alan-null]

I can take a look this week and create PR.

[farag2]

#430

[alan-null]

Improvement will be implemented here.