itm4n/PrivescCheck

Get-ScheduledTaskList does not work in Powershell V2

SAERXCIT opened this issue ยท 5 comments

Hi @itm4n !

The function Get-ScheduledTaskList does not work in Powershell V2:

Screenshot from 2021-02-09 11-41-43

This seems to be due to the use of member enumeration on lines 2496 and 2511, a feature only available starting from PS v3 ($TaskExec is of type XmlElementList, an array of XmlElement objects):

PrivescCheck/PrivescCheck.ps1

Lines 2496 to 2511 in 39fc098

$TaskCommandLine = "$($TaskExec.Command) $($TaskExec.Arguments)"
$Principal = $TaskXml.GetElementsByTagName("Principal")
$CurrentUserIsOwner = $False
if ($Principal.UserId) {
$PrincipalName = Convert-SidToName -Sid $Principal.UserId
if ($(Invoke-UserCheck).SID -eq $Principal.UserId) {
$CurrentUserIsOwner = $True
}
} elseif ($Principal.GroupId) {
$PrincipalName = Convert-SidToName -Sid $Principal.GroupId
}
if ($TaskExec.Command.Length -gt 0) {

Thanks for your work !

The $Principal variable is also subject to the same issue, which leads to the RunAs and CurrentUserIsOwner element not being filled out properly, leading to checks Invoke-ScheduledTasksImagePermissionsCheck and Invoke-ScheduledTasksUnquotedPathCheck showing false positives.

I opened PR #9 to fix this issue, feel free to merge or to implement another solution ๐Ÿ™‚

itm4n commented

You made a really good point once again. ๐Ÿ™‚
I wonder how many similar bugs are lying in my code now... Perhaps it's time for me to start implementing some unit tests. ๐Ÿ™„

I tested your solution before merging the code but it triggered exceptions in PSv2 whenever it tried to access the UserId attribute of a Group Principal (whereas it was just fine in PSv5). Therefore I had to implement your solution slightly differently. I tested the code in PSv5 and PSv2, I get the exact same result now without any errors.

Thanks again for your contribution. It is very much appreciated.
May I ask you to check if this new version works for you too?

It's working the same on both versions for me too now, great, thanks for taking this into account ! I'm closing the PR.

I do have one interrogation though. When translating from member enumeration to Select-Object -ExpandProperty I aimed to emulate the same behaviour as upstream without causing regressions. But what is the behaviour of upstream when multiple <Exec> actions are defined in the scheduled tasks? Or multiple <Principal>? Is it even possible, should we even worry about this? I don't know enough about Windows Scheduled Tasks to say if it is.

I welcome your input on this ๐Ÿ™‚

itm4n commented

As far as I can tell, there is only one Principal per Task. However, there can be multiple Actions, and these are not even necessarily Exec nodes. That's a potential issue, for sure.

According to the XML schema provided by MS here:

<!-- Principals -->
<xs:complexType name="principalsType">
    <xs:sequence>
        <xs:element name="Principal" type="principalType" maxOccurs="1" />
    </xs:sequence>
</xs:complexType>

Although the node is called Principals, there can be at most one Principal element inside (maxOccurs="1").

<xs:complexType name="actionsType">
    <xs:sequence>
        <xs:group ref="actionGroup" maxOccurs="32" />
    </xs:sequence>
    <xs:attribute name="Context" type="xs:IDREF" use="optional" />
</xs:complexType>

There can be up to 32 Actions per Task.

<xs:group name="actionGroup">
    <xs:choice>
        <xs:element name="Exec"          type="execType" />
        <xs:element name="ComHandler"    type="comHandlerType" />
        <xs:element name="SendEmail"     type="sendEmailType" />
        <xs:element name="ShowMessage"   type="showMessageType" />
    </xs:choice>
</xs:group>

These Actions can be something other than a simple Exec node.
So, yeah, I should take this into account... :/

itm4n commented

Issue fixed, thanks again for your help!