Some Bugs and Issues and Enhancements

asterictnl-lvdw commented 2 years ago

Hello,

I found some bugs and enhancements that could be done:

Bugs:

There is a bug with the Connect-SPOService that the orgname does not correctly be inputted in the URL. FIX:
```
Write-Output "Connecting to SharePoint Service"
    Connect-SPOService -Url https://$org_name-admin.sharepoint.com
```
Not using "" because this is not needed.

Enhancements:

Add a new module (this does not have to be installed when the other modules are installed! This is for Security & Compliance where also audits are available:
```
Write-Output "Connecting to IPPSSession..."
    Connect-IPPSSession
```
Add the feature to define the script folder. Now you are required to execute the M365Inspector tool within the folder and the Inspectors folder must be alongside.

ThoughtContagion commented 2 years ago

Thank you, I do like the addition of the Security and Compliance Center for additional visibility and inspectors - we can certainly add that and begin building out new features based upon that. We can look into adding a custom script path as well for Inspector execution. We can also remove the quotes from the URL string, but we have not seen any issues with the organization name not being input correctly, could you provide an example or error logs to show when/how that failed?

asterictnl-lvdw commented 2 years ago

There are some more multiple bugs I found after fixing some issues: I can't get a specific script to work. It seems to throw this error:

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Out-File : Cannot perform operation because the wildcard path C:\AsterM365InspectorTool\Out\[VULNERABLE] - Tenant contains users without MFA. Considering 
enabling MFA for all users. did not resolve to a file.
At line:145 char:31
+ ...        $finding.AffectedObjects | Out-File -FilePath $out_path\$fname
+                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : OpenError: (C:\AsterM365Ins... for all users.:String) [Out-File], FileNotFoundException
    + FullyQualifiedErrorId : FileOpenFailure,Microsoft.PowerShell.Commands.OutFileCommand
Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Cannot find an overload for "Replace" and the argument count: "2".
At line:150 char:5
+ ...             $affected_object_html += $templates.AffectedObjectsTempla ...
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodException
    + FullyQualifiedErrorId : MethodCountCouldNotFindBest

Compress-Archive : The archive file C:\AsterM365InspectorTool\Out\astercomputers_Report.zip already exists. Use the -Update parameter to update the existing 
archive file or use the -Force parameter to overwrite the existing archive file.
At line:200 char:3
+   Compress-Archive @compress
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (C:\AsterM365Ins...ters_Report.zip:String) [Compress-Archive], IOException
    + FullyQualifiedErrorId : ArchiveFileExists,Compress-Archive

This is the error that also breaks the script. I do not know if it is because of the script itself.

But It happens when I have this script ran:

function Audit-GlobalAdmins{
$rolegba = Get-MsolRole -RoleName "Company Administrator" 
$count = (Get-MsolRoleMember -RoleObjectId $rolegba.objectid)
if ($count.Count -ile 1 -or $count.Count -igt 4){
return $count.ToString()
}
return $null
}
return Audit-GlobalAdmins

It does output correctly when not running the inspector.

asterictnl-lvdw commented 2 years ago

[UPDATE 15-03-2022: 16:50 GMT+1] Here a list of bugs I found when the report was generated:

It seems that the first object get printed out twice. Not sure if this is because of me. But I have to look further into it.
Affected O365 Objects: frequently misses values or seem to have weird values. This is tested with custom inspectors. I do not know what the result would be with the regular inspectors and what their expected output would be. I need to research that as well. I made the custom inspectors conform the template that you presented.

ThoughtContagion commented 2 years ago

Would you be able to give me the name of the finding for the first result? This changes based on the tenant's configuration, but if one of the inspectors is causing a duplicate value to be returned that should be an easy fix, or I may be able to explain why it appears to be a duplicate.

ThoughtContagion commented 2 years ago

There are some more multiple bugs I found after fixing some issues: I can't get a specific script to work. It seems to throw this error:
Cannot find an overload for "Replace" and the argument count: "2".
====================Truncated=================
This is the error that also breaks the script. I do not know if it is because of the script itself.

But It happens when I have this script ran:
function Audit-GlobalAdmins{
$rolegba = Get-MsolRole -RoleName "Company Administrator" 
$count = (Get-MsolRoleMember -RoleObjectId $rolegba.objectid)
if ($count.Count -ile 1 -or $count.Count -igt 4){
return $count.ToString()
}
return $null
}
return Audit-GlobalAdmins
It does output correctly when not running the inspector.

This function is already included in the tool under the ProperAdminCount.ps1 script - https://github.com/soteria-security/365Inspect/blob/main/Inspectors/ProperAdminCount.ps1

You are seeing this error because your script returns a System.Object array. You're casting an array of object properties to a string and overloading the replace switch in the conversion to the html report. You would want to pick a property to return - eg, $count.DisplayName

ThoughtContagion commented 2 years ago

Thank you!

I have merged the PR with the changes to the 365Inspect script to allow for the Security Center, a handful of inspectors will be inbound in the coming weeks to make use of that functionality.

At this time, we feel like a custom script path may introduce added complexity and will not be adding that to the base tool. This may change in the future.

The issues listed above also appear to be a result of the custom script written to include in the local copy of the tool in your environment, and not an issue with any of the existing scripts or the tool itself.

soteria-security / 365Inspect

Some Bugs and Issues and Enhancements #26