itm4n / PrivescCheck

Privilege Escalation Enumeration Script for Windows
BSD 3-Clause "New" or "Revised" License
2.79k stars 416 forks source link

Invoke-ScheduledTasks*Check: Duplicated/missing results when multiple results exist for the same task #12

Closed SAERXCIT closed 3 years ago

SAERXCIT commented 3 years ago

Hi !

The scheduled tasks checks (Invoke-ScheduledTasksImagePermissionsCheck, Invoke-ScheduledTasksUnquotedPathCheck) do not show correct results when multiple results exist for the same task: the first Add-Members work as expected, but further Add-Members for subsequent results of the same CurrentTask do not work, as the function Add-Member does not allow overwriting by default. This results in the ModifiablePath, IdentityReference, and Permissions elements not being updated (and errors in the console), which for the user means the same result is outputted multiple times.

Example on an up-to-date 20H2, PSv5 (same ModifiablePath, IdentityReference, and Permissions for both items; check was executed as a privileged user for demonstration purposes, so technically not a false positive :slightly_smiling_face:): Screenshot from 2021-02-15 10-43-58

Proposed solution: adding -Force to each Add-Member of both these checks. Example output (the attributes are updated, and no more error outputted): Screenshot from 2021-02-15 10-43-40

Cheers! :slightly_smiling_face:

itm4n commented 3 years ago

That's awesome! You are finding all the bugs in my code! 🙂

This one was easy to fix though. Here is what I did:

$ResultItem = $CurrentTask.PsObject.Copy()
$ResultItem | Add-Member -MemberType "NoteProperty" -Name "ModifiablePath" -Value $_.ModifiablePath
$ResultItem | Add-Member -MemberType "NoteProperty" -Name "IdentityReference" -Value $_.IdentityReference
$ResultItem | Add-Member -MemberType "NoteProperty" -Name "Permissions" -Value $_.Permissions
$ResultItem

Rather than modifying the source object, I create a copy of it and I add the relevant fields to the object's copy. I made sure that it is still PSv2 compatible because I haven't used this trick before. In the other checks, I created a new object for each new result. I don't know why I didn't do that in these two checks.

Anyway, thanks a lot! 🙂

itm4n commented 3 years ago

And, yeah the -Force would have been a quick fix but it could have some side effects that are difficult to foresee. So, I prefer to play safe here and simply make a copy of the object. Perhaps it's actually what -Force does under the hood... 🙄 I don't know.

SAERXCIT commented 3 years ago

Great, thanks for fixing!

I saw Copy() but did not want to implement it because its documentation page mentions it only applies to PSv5, but since you actually tried and it works, it means the MS doc is not complete (or I misunderstood something :upside_down_face:).

Closing the issue, cheers!

itm4n commented 3 years ago

Yup, we read the same doc! 😉 I think that PSv5.1 is now considered as the minimum PS version in the entire documentation. But a Copy() seemed like a very basic operation that you would expect even in older versions. So, I tested it by copying a random object in PSv2 and it worked! 🙂 I double checked on Windows Server 2008 R2 as well.