fix: guard null stream/reader/writer in PE subsys functions - #6711
Conversation
Get-PESubsystem/Set-PESubsystem could error when the constructor threw, because the finally block unconditionally called Close() on $null. Add null checks, and also close $binaryWriter in Set-PESubsystem. Fixes install shim errors like: You cannot call a method on a null-valued expression.
Summary by CodeRabbit
WalkthroughChangesPE subsystem resource cleanup
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/core.ps1 (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd the BOM required by PSScriptAnalyzer.
PSScriptAnalyzer reports
PSUseBOMForUnicodeEncodedFileforlib/core.ps1. Save the file with a BOM to clear the warning and preserve its Unicode encoding.Source: Linters/SAST tools
🧹 Nitpick comments (1)
lib/core.ps1 (1)
43-45: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winClose the shared stream in ownership order.
$binaryReaderand$binaryWriterboth wrap$fileStream, and eachClose()closes its underlying stream. Close$binaryWriterfirst, then$binaryReader, and$fileStreamlast. Alternatively, construct the wrappers withleaveOpen = $trueand close$fileStreamonce. (learn.microsoft.com)Proposed cleanup order
finally { + if ($binaryWriter) { $binaryWriter.Close() } if ($binaryReader) { $binaryReader.Close() } if ($fileStream) { $fileStream.Close() } - if ($binaryWriter) { $binaryWriter.Close() } }
1 participant
Fix null-reference errors when creating shims during install.
Get-PESubsystemandSet-PESubsystemunconditionally called.Close()on stream/reader/writer objects in theirfinallyblocks. If the constructor threw (e.g. file missing or unreadable), those variables were still $null, causing:You cannot call a method on a null-valued expression.
This commit adds null checks and also closes $binaryWriter in
Set-PESubsystem, which was previously leaked.