PowerShell icon indicating copy to clipboard operation
PowerShell copied to clipboard

PSInvalidCastException should not display the value to be cast when target type is a secure string

Open isra-fel opened this issue 1 year ago • 11 comments

Prerequisites

Steps to reproduce

If you have a template parameter with a secure string type and you accidentally pass a plain string, on the command line, that string is prnted in the error text. Since you know it's a secure string, it shoud not be printed in the error

Example 1:

PS C:\> ConvertFrom-SecureString "p@assw0rd"
ConvertFrom-SecureString: Cannot bind parameter 'SecureString'. Cannot convert the "p@assw0rd" value of type "System.String" to type "System.Security.SecureString"

Example 2:

PS> New-AzResourceGroupDeployment -ResourceGroupName MyRG -Name MyDeployment -TemplateParameterFile .\mytemplate.parameters.json -TemplateFile .\mytemplate.json -domainPassword $PlainString -Whatif

# Template file contains parameter: 
#         "domainPassword": {
#            "type": "securestring"
#        }

New-AzResourceGroupDeployment: Cannot bind parameter 'domainPassword'. Cannot convert the "p@assw0rd" value of type "System.String" to type "System.Security.SecureString".

# the value of "p@ssw0rd" should not be revealed.

Originally reported by @dbaileyut in https://github.com/Azure/azure-powershell/issues/21250

Expected behavior

the value of "p@ssw0rd" should not be revealed.

Actual behavior

the value of "p@ssw0rd" was leaked to console.

Error details

No response

Environment data

Name                           Value
----                           -----
PSVersion                      7.3.3
PSEdition                      Core
GitCommitId                    7.3.3
OS                             Microsoft Windows 10.0.19045
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visuals

No response

isra-fel avatar Mar 21 '23 05:03 isra-fel

Accidently closed and reopened the issue. Needs-Triage label was gone...

isra-fel avatar Mar 21 '23 05:03 isra-fel

Since you know it's a secure string, it shoud not be printed in the error

You can not type System.Secure.SecureString as plain text.

Doesn't it bother you that the command is saved in the history (Get-PSReadlineOption).HistorySavePath and in the logs, if they are configured?

237dmitry avatar Mar 21 '23 06:03 237dmitry

This is a generic error message - when a value cannot be converted to a destination type the message is "cannot convert [the value converted to string] to [destination type name]"

The examples in the OP are not good examples the error detail would contain the line of text containing the password

InvocationInfo        :
    MyCommand        : ConvertFrom-SecureString
    ScriptLineNumber : 1
    OffsetInLine     : 27
    HistoryId        : 578
    Line             :  ConvertFrom-SecureString "p@assw0rd"
    PositionMessage  : At line:1 char:27
                       +  ConvertFrom-SecureString "p@assw0rd"
                       +                           ~~~~~~~~~~~
    CommandOrigin    : Internal

This is NOT shown PowerShell 7's default concise view but if $errorview is set to normal , and on older versions of PowerShell the error would be

>  ConvertFrom-SecureString "p@assw0rd"
ConvertFrom-SecureString : Cannot bind parameter 'SecureString'. Cannot convert the "p@assw0rd" value of type "System.String" to type "System.Security.SecureString".
At line:1 char:27
+  ConvertFrom-SecureString "p@assw0rd"
+                           ~~~~~~~~~~~
+ CategoryInfo          : InvalidArgument: (:) [ConvertFrom-SecureString], ParameterBindingException
+ FullyQualifiedErrorId : CannotConvertArgumentNoMessage,Microsoft.PowerShell.Commands.ConvertFromSecureStringCommand

As @237dmitry says in both these cases the filter which prevents plaintext passwords being saved to the history would not be triggered. so the password would be saved.

however if I write

$password = Get-MyPasswordAsPlainText
Invoke-Something -password MyPassword

The password isn't visible and even the command to get it is filtered out of history logging, but if Invoke-Something demands a secure string, my mistake puts the plain text password in the error message.

The ideal solution would be that when an Invalid Cast exception is thrown a check is done for converting to secure string and different error is thrown. This would be an enhancement.

@isra-fel I'm going to flag this as an enhancement. Would you please change the the title to something like "Invalid cast exception should not display the value to be cast when target type is a secure string"

jhoneill avatar Mar 21 '23 11:03 jhoneill

There is default solution about entering password/ Read-Host -AsSecureString

$pass = Read-Host "Enter password" -AsSecureString
$pass    # System.Security.SecureString
ConvertFrom-SecureString $pass -Asplaintext

This is the basic example. You can encrypt data with password to view it on different computers and platforms. With self-decrypted scripts.

237dmitry avatar Mar 21 '23 12:03 237dmitry

@237dmitry

This is happening because the user doesn't know a secure string is required, and therefore passes a normal string. If they know the input needs to be a secure string the are multiple ways they can get one.

jhoneill avatar Mar 21 '23 14:03 jhoneill

@jhoneill glad you understand why I submitted the original issue/enhancement. The full context is that I did something admittedly silly by passing a [string] to a [securestring] parameter like this:

$Cred = Get-Credenital
New-AzResourceGroupDeployment -ResourceGroupName MyRG -Name MyDeployment -TemplateParameterFile .\mytemplate.parameters.json -TemplateFile .\mytemplate.json -domainPassword $Cred.GetNetworkCredential().Password -Whatif

Then, I was surprised when the password was printed to the console.

dbaileyut avatar Mar 21 '23 16:03 dbaileyut

Reasonable request I think. Opening to the Engine WG for triage

SeeminglyScience avatar Mar 21 '23 17:03 SeeminglyScience

The WG discussed this and believe this as a security issue, and it should be addressed. @PowerShell/area-security do you have any suggestions as to what the actual message should be? For example, do we just leave out the offending term or replace the characters with * or ??

JamesWTruher avatar Apr 03 '23 21:04 JamesWTruher

I don't see this as a security issue ... the ConvertFrom-SecureString cmdlet is being misused and there is no reason to think the input is sensitive data. There are many ways to inadvertently leak sensitive data, and care should always be taken. But I agree that the error message should not contain the input parameter argument just to be safe. I would simply not include the parameter argument in this specific case.

PaulHigin avatar Apr 03 '23 22:04 PaulHigin

@PaulHigin do you believe that the error message of [SecureString]"plaintext" should be altered?

> [securestring]"happy"                    
InvalidArgument: Cannot convert the "happy" value of type "System.String" to type "System.Security.SecureString".

this would occur when the user has a parameter of type securestring and the user provides a string as a parameter value.

JamesWTruher avatar Apr 03 '23 22:04 JamesWTruher

~~My vote would be replacing with * but it's not a strong opinion.~~ Actually, it's probably better to leave the value out as someone might try to dig for the literal string of asterisks.

dbaileyut avatar Apr 10 '23 15:04 dbaileyut

WG-Security: The group discussed this and agree that for SecureString types, any conversion error should not include the plain text string being converted. We also feel this would apply to the PSCredential type.

PaulHigin avatar Apr 24 '23 20:04 PaulHigin

This is also a security risk when running deployment jobs inside in Azure Devops/Github Action agents, where the conversion also leaks the sensitive data in the exception message when looking at the logs.

I was recently burnt by this when upgrading Microsoft.Graph to the latest release, which contained breaking changes that converted AccessToken parameter type from string to SecureString in the Connect-MgGraph cmdlet, and the whole plaintext token was exposed in the pipeline logs.

ArmaanMcleod avatar Jul 16 '23 00:07 ArmaanMcleod