1
\$\begingroup\$

I have written a script for CVE-2023-24932 like below. But I want to improve my script. I am open to new ideas.

Here is my script:

$DBXUpdateSuccess = Get-EventLog -LogName System -Source "Microsoft-Windows-TPM-WMI" -InstanceId 1035 -ErrorAction SilentlyContinue

if ($DBXUpdateSuccess){
    Write-Host  "Patch has been Applied Successfully"  
    Exit 0
}

$registryKey = "HKLM:\SYSTEM\CurrentControlSet\Control\SecureBoot\"
$fileToCopy = "C:\Windows\System32\SecureBootUpdates\SKUSiPolicy.p7b"
$destination = "B:\EFI\Microsoft\SKUSiPolicy.p7b"
$logFile = "C:\Helpdesk\WU052023.log"

# Check if the logfile exists meaning script has already completed once.
if (Test-Path $logFile) {
    Write-Host "Manual steps have completed once, exiting."
    Exit 0
}
Else{
    Write-Host "05-2023 manual steps required."
}

# Check if the file SKUSiPolicy.p7b exists, meaning 05-2023 update has been installed
if (Test-Path $fileToCopy) {
    Write-Host "05-2023 update has been installed"
}
Else{
    Write-Host "05-2023 update needs to be installed."
    exit 1
}

# Check if AvailableUpdates registry key is 0
$availableUpdates = (Get-ItemProperty -Path $registryKey).AvailableUpdates
if ($availableUpdates -eq 0) {
    Write-Host "Registry key AvailableUpdates is 0."
} elseif ($availableUpdates -eq 0x10) {
    Write-Host "Registry key AvailableUpdates is 0x10. Manual steps pending. Reboot."
    exit 0
} else {
    Write-Host "Registry key AvailableUpdates is in an unknown state."
    exit 11
}

Write-Host "Mounting EFI volume to B:"
# Mount the EFI volume to drive B:
$mountResult = mountvol B: /S
if ($mountResult -ne $null) {
    Write-Host "EFI mount failed."
    exit 2
}

# Copy the file to EFI volume
Write-Host "Copying file"
Copy-Item -Path $fileToCopy -Destination $destination -Force
# Verify if the file exists in B:\EFI\Microsoft\
if (Test-Path $destination) {
    Write-Host "The file copy was successful."
    # Dismount B:
    mountvol B: /D
} else {
    Write-Host "File copy failed."
    exit 3
}

# Set the AvailableUpdates registry entry to 0x10
Write-Host "Setting registry key AvailableUpdates to 0x10."
Set-ItemProperty -Path $registryKey -Name "AvailableUpdates" -Value 0x10 -Type DWORD
$availableUpdates = (Get-ItemProperty -Path $registryKey).AvailableUpdates
If ($availableUpdates -eq 0x10) {
    Write-Host "Registry key AvailableUpdates is 0x10. 05-2023 manual steps are complete."
}
Else{
    Write-Host "Registry key AvailableUpdates is NOT 0x10. Registry set falied"
    exit 4
}

# Write the date and time to the log file. This file's existence will stop further runs of the script.
(Get-Date).ToString("yyyy-MM-dd HH:mm:ss") | Out-File -FilePath $logFile -Append

Write-Host "A reboot is required."
Write-Host "After reboot, wait 5 minutes then check System Events for ID 1035 'Secure Boot Dbx update applied successfully' and reboot again to complete."

$Process = "C:\windows\system32\shutdown.exe"
$ShutdownArgs = '/r /f /t 0 /c "Apply fix for CVE-2023-24932"'
Start-Process $Process -ArgumentList $ShutdownArgs -NoNewWindow

exit 0
\$\endgroup\$
1
  • 3
    \$\begingroup\$ The post title should simply state what your app does, not the review request. What's CVE-2023-24932 for those who don't know? Thanks. \$\endgroup\$
    – ggorlen
    Commented May 16, 2023 at 5:03

1 Answer 1

1
\$\begingroup\$

I'm not familiar with the CVE you mentioned or its remediation steps so I'm just going to assume the business logic works.

Two things I'd note:

  • Use return instead of exit

Your script just wants to exit and return some value to the caller, not necessarily to terminate the entire powershell session it is running in, so exit should be replaced by return.

  • Do not use Write-Host until you explicitly want to write to the host console

Write-Host is meant to display a message to console only, whereas Write-Output writes to your standard output, which is what you're manipulating and passing around when you use piping or the redirection operator (like & ./myscript.ps1 > outputfile.log).

Using Write-Host tells powershell "here, show this to the user", whereas Write-Output tells powershell "here's my output, I'll let you handle that", and allows powershell to pipe that output around, format it (manually via Format-... or automatically), write it to a file, write it to a job output, etc... And if you don't specify anything, it'll do the same thing as Write-Host anyway.

  • Bonus thing: use PascalCase

PascalCase (no word separator, all words capitalized including the first) is the standard recommended by Microsoft for naming variables and commands. Powershell is case-insensitive for the most part but that doesn't mean you can't make it look good. Also it's worth noting that Microsoft docs use the Allman indenting style but there aren't any explicit guidelines for that (and most people seem to use K&R).


You probably want to check out this document which has tons of stuff and that is worth a read even if you don't intend to apply everything: Microsoft Learn - Strongly Encouraged Development Guidelines. There aren't a ton of things applicable to your script but since your question is about best practices...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.