Skip to content

Fix VB compiler crashes when decoding attributes #61537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AlekseyTs merged 7 commits into from
Jun 13, 2022
Merged

Fix VB compiler crashes when decoding attributes #61537

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
05c5780
Fix VB compiler crashes when decoding attributes
Youssef1313 May 26, 2022
69ac80e
Simplify per review comment
Youssef1313 May 27, 2022
8d30980
More crash fixes
Youssef1313 May 27, 2022
469bb32
Address review comments
Youssef1313 Jun 7, 2022
555b2ce
Update AttributeTests_StructLayout.vb
Youssef1313 Jun 7, 2022
f702ee7
Refactor
Youssef1313 Jun 12, 2022
61a1e1f
Fix typo
Youssef1313 Jun 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 34 additions & 26 deletions src/Compilers/VisualBasic/Portable/Symbols/Attributes/AttributeData.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,16 +243,28 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
diagnostics.Add(If(nodeOpt IsNot Nothing, nodeOpt.Name.GetLocation, NoLocation.Singleton), useSiteInfo)
End If

' BC31205: First argument to a security attribute must be a valid SecurityAction
diagnostics.Add(ErrorFactory.ErrorInfo(ERRID.ERR_SecurityAttributeMissingAction,
Me.AttributeClass),
' BC31211: First argument to a security attribute must be a valid SecurityAction
diagnostics.Add(ErrorFactory.ErrorInfo(ERRID.ERR_SecurityAttributeMissingAction),
If(nodeOpt IsNot Nothing, nodeOpt.Name.GetLocation, NoLocation.Singleton))

hasErrors = True

Return Nothing
End Function

Private Shared Function GetArgumentAndLocation(nodeOpt As AttributeSyntax, value As Integer) As (Argument As String, Location As Location)

@AlekseyTs AlekseyTs May 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetArgumentAndLocation

GetFirstArgumentDisplayAndLocation? #Closed

@AlekseyTs AlekseyTs May 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argument

ArgumentDisplay? #Closed

If nodeOpt IsNot Nothing Then
If nodeOpt.ArgumentList IsNot Nothing AndAlso nodeOpt.ArgumentList.Arguments.Count > 0 Then
Dim arg = nodeOpt.ArgumentList.Arguments(0)
Return (arg.ToString(), arg.GetLocation())
Else
Return (value.ToString(), nodeOpt.GetLocation())
End If
Else
Return ("", NoLocation.Singleton)
End If
End Function

Private Function ValidateSecurityAction(
typedValue As TypedConstant,
targetSymbol As Symbol,
Expand All @@ -271,10 +283,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
DeclarativeSecurityAction.LinkDemand

If Me.IsTargetAttribute(targetSymbol, AttributeDescription.PrincipalPermissionAttribute) Then
' BC31209: SecurityAction value '{0}' is invalid for PrincipalPermission attribute
diagnostics.Add(ERRID.ERR_PrincipalPermissionInvalidAction,
If(nodeOpt IsNot Nothing, nodeOpt.ArgumentList.Arguments(0).GetLocation(), NoLocation.Singleton),
If(nodeOpt IsNot Nothing, nodeOpt.ArgumentList.Arguments(0).ToString(), ""))
' BC31215: SecurityAction value '{0}' is invalid for PrincipalPermission attribute
Dim valueLocation = GetArgumentAndLocation(nodeOpt, securityAction)

@AlekseyTs AlekseyTs May 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dim

Please spell out the type. We are not using implicit typing in declarations when the type isn't obvious from the initial value (i.e. not somehow spelled out there). #Closed

@AlekseyTs AlekseyTs May 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valueLocation

displayAndLocation? #Closed

diagnostics.Add(ERRID.ERR_PrincipalPermissionInvalidAction, valueLocation.Location, valueLocation.Argument)

hasErrors = True
Return DeclarativeSecurityAction.None
Expand All @@ -301,11 +312,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
isPermissionRequestAction = True

Case Else
' BC31206: Security attribute '{0}' has an invalid SecurityAction value '{1}'
diagnostics.Add(ERRID.ERR_SecurityAttributeInvalidActionTypeOrMethod,
If(nodeOpt IsNot Nothing, nodeOpt.ArgumentList.Arguments(0).GetLocation(), NoLocation.Singleton),
If(nodeOpt IsNot Nothing, nodeOpt.Name.ToString, ""),
If(nodeOpt IsNot Nothing, nodeOpt.ArgumentList.Arguments(0).ToString(), ""))
' BC31214: SecurityAction value '{0}' is invalid for security attributes applied to a type or a method.
Dim valueLocation = GetArgumentAndLocation(nodeOpt, securityAction)
diagnostics.Add(ERRID.ERR_SecurityAttributeInvalidActionTypeOrMethod, valueLocation.Location, valueLocation.Argument)

hasErrors = True
Return DeclarativeSecurityAction.None
Expand All @@ -315,10 +324,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
If targetSymbol.Kind = SymbolKind.NamedType OrElse targetSymbol.Kind = SymbolKind.Method Then
' Types and methods cannot take permission requests.

' BC31208: SecurityAction value '{0}' is invalid for security attributes applied to a type or a method
diagnostics.Add(ERRID.ERR_SecurityAttributeInvalidActionTypeOrMethod,
If(nodeOpt IsNot Nothing, nodeOpt.ArgumentList.Arguments(0).GetLocation, NoLocation.Singleton),
If(nodeOpt IsNot Nothing, nodeOpt.ArgumentList.Arguments(0).ToString(), ""))
' BC31214: SecurityAction value '{0}' is invalid for security attributes applied to a type or a method.
Dim valueLocation = GetArgumentAndLocation(nodeOpt, securityAction)
diagnostics.Add(ERRID.ERR_SecurityAttributeInvalidActionTypeOrMethod, valueLocation.Location, valueLocation.Argument)

hasErrors = True
Return DeclarativeSecurityAction.None
Expand All @@ -327,10 +335,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
ElseIf targetSymbol.Kind = SymbolKind.Assembly Then
' Assemblies cannot take declarative security.

' BC31207: SecurityAction value '{0}' is invalid for security attributes applied to an assembly
diagnostics.Add(ERRID.ERR_SecurityAttributeInvalidActionAssembly,
If(nodeOpt IsNot Nothing, nodeOpt.ArgumentList.Arguments(0).GetLocation, NoLocation.Singleton),
If(nodeOpt IsNot Nothing, nodeOpt.ArgumentList.Arguments(0).ToString(), ""))
' BC31213: SecurityAction value '{0}' is invalid for security attributes applied to an assembly.
Dim valueLocation = GetArgumentAndLocation(nodeOpt, securityAction)
diagnostics.Add(ERRID.ERR_SecurityAttributeInvalidActionAssembly, valueLocation.Location, valueLocation.Argument)

hasErrors = True
Return DeclarativeSecurityAction.None
Expand Down Expand Up @@ -376,7 +383,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols

If resolvedFilePath Is Nothing Then

' BC31210: Unable to resolve file path '{0}' specified for the named argument '{1}' for PermissionSet attribute
' BC31216: Unable to resolve file path '{0}' specified for the named argument '{1}' for PermissionSet attribute.
Dim argSyntaxLocation As Location = If(arguments.AttributeSyntaxOpt IsNot Nothing,
arguments.AttributeSyntaxOpt.ArgumentList.Arguments(1).GetLocation(),
NoLocation.Singleton)
Comment on lines 399 to 401

@Youssef1313 Youssef1313 May 27, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to write a test that makes a crash here, so I didn't make a change.
In case someone was able to, let me know.

Expand Down Expand Up @@ -429,14 +436,16 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
Case ClassInterfaceType.None, Cci.Constants.ClassInterfaceType_AutoDispatch, Cci.Constants.ClassInterfaceType_AutoDual
Exit Select
Case Else
diagnostics.Add(ERRID.ERR_BadAttribute1, If(nodeOpt IsNot Nothing, nodeOpt.ArgumentList.Arguments(0).GetLocation(), NoLocation.Singleton), Me.AttributeClass)
Dim location = GetArgumentAndLocation(nodeOpt).Location
diagnostics.Add(ERRID.ERR_BadAttribute1, location, Me.AttributeClass)
End Select
End Sub

Friend Sub DecodeInterfaceTypeAttribute(node As AttributeSyntax, diagnostics As BindingDiagnosticBag)
Dim discarded As ComInterfaceType = Nothing
If Not DecodeInterfaceTypeAttribute(discarded) Then
diagnostics.Add(ERRID.ERR_BadAttribute1, node.ArgumentList.Arguments(0).GetLocation(), Me.AttributeClass)
Dim location = GetArgumentAndLocation(nodeOpt).Location
diagnostics.Add(ERRID.ERR_BadAttribute1, location, Me.AttributeClass)
End If
End Sub

Expand Down Expand Up @@ -477,9 +486,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
' Native compiler allows only a specific GUID format: "D" format (32 digits separated by hyphens)
Dim guidVal As Guid
If Not Guid.TryParseExact(guidString, "D", guidVal) Then
diagnostics.Add(ERRID.ERR_BadAttributeUuid2,
If(nodeOpt IsNot Nothing, nodeOpt.ArgumentList.Arguments(0).GetLocation(), NoLocation.Singleton),
Me.AttributeClass, If(guidString, ObjectDisplay.NullLiteral))
Dim location = GetArgumentAndLocation(nodeOpt).Location
diagnostics.Add(ERRID.ERR_BadAttributeUuid2, location, Me.AttributeClass, If(guidString, ObjectDisplay.NullLiteral))
End If
End Sub

Expand Down
12 changes: 11 additions & 1 deletion src/Compilers/VisualBasic/Portable/Symbols/Source/SourceMethodSymbol.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1651,7 +1651,17 @@ lReportErrorOnTwoTokens:

Dim moduleName As String = TryCast(attrData.CommonConstructorArguments(0).ValueInternal, String)
If Not MetadataHelpers.IsValidMetadataIdentifier(moduleName) Then
diagnostics.Add(ERRID.ERR_BadAttribute1, arguments.AttributeSyntaxOpt.ArgumentList.Arguments(0).GetLocation(), attrData.AttributeClass)
Dim location As Location
If arguments.AttributeSyntaxOpt IsNot Nothing Then
If arguments.AttributeSyntaxOpt.ArgumentList IsNot Nothing AndAlso arguments.AttributeSyntaxOpt.ArgumentList.Arguments.Count > 0 Then
location = arguments.AttributeSyntaxOpt.ArgumentList.Arguments(0).GetLocation()
Else
location = arguments.AttributeSyntaxOpt.GetLocation()
End If
Else
location = NoLocation.Singleton
End If
diagnostics.Add(ERRID.ERR_BadAttribute1, location, attrData.AttributeClass)
End If

' Default value of charset is inherited from the module (only if specified).
Expand Down
Loading