Skip to content

Fix NRE in XamlLoadPermission.FromXml for element without class attribute #214

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

Closed
hughbe wants to merge 2 commits into from
Closed

Fix NRE in XamlLoadPermission.FromXml for element without class attribute #214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5648a7
Cleanup XamlLoadPermission and XamlAcessLevel
hughbe Dec 26, 2018
2479ee0
Fix NullReferenceException parsing element without `class` attribute
hughbe Dec 26, 2018
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
48 changes: 27 additions & 21 deletions src/Microsoft.DotNet.Wpf/src/System.Xaml/System/Xaml/Permissions/XamlAccessLevel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ namespace System.Xaml.Permissions
[Serializable]
public class XamlAccessLevel
{
private const string XamlAccessLevelTagName = "XamlAccessLevel";
private const string AssemblyNameAttributeName = "AssemblyName";
private const string TypeNameAttributeName = "TypeName";

private XamlAccessLevel(string assemblyName, string typeName)
{
AssemblyNameString = assemblyName;
Expand All @@ -27,6 +31,7 @@ public static XamlAccessLevel AssemblyAccessTo(Assembly assembly)
{
throw new ArgumentNullException(nameof(assembly));
}

return new XamlAccessLevel(assembly.FullName, null);
}

Expand All @@ -36,7 +41,8 @@ public static XamlAccessLevel AssemblyAccessTo(AssemblyName assemblyName)
{
throw new ArgumentNullException(nameof(assemblyName));
}
ValidateAssemblyName(assemblyName, "assemblyName");

ValidateAssemblyName(assemblyName, nameof(assemblyName));
return new XamlAccessLevel(assemblyName.FullName, null);
}

Expand All @@ -46,6 +52,7 @@ public static XamlAccessLevel PrivateAccessTo(Type type)
{
throw new ArgumentNullException(nameof(type));
}

return new XamlAccessLevel(type.Assembly.FullName, type.FullName);
}

Expand All @@ -55,6 +62,7 @@ public static XamlAccessLevel PrivateAccessTo(string assemblyQualifiedTypeName)
{
throw new ArgumentNullException(nameof(assemblyQualifiedTypeName));
}

int nameBoundary = assemblyQualifiedTypeName.IndexOf(',');
if (nameBoundary < 0)
{
Expand All @@ -64,7 +72,7 @@ public static XamlAccessLevel PrivateAccessTo(string assemblyQualifiedTypeName)
string typeName = assemblyQualifiedTypeName.Substring(0, nameBoundary).Trim();
string assemblyFullName = assemblyQualifiedTypeName.Substring(nameBoundary + 1).Trim();
AssemblyName assemblyName = new AssemblyName(assemblyFullName);
ValidateAssemblyName(assemblyName, "assemblyQualifiedTypeName");
ValidateAssemblyName(assemblyName, nameof(assemblyQualifiedTypeName));

return new XamlAccessLevel(assemblyName.FullName, typeName);
}
Expand All @@ -78,7 +86,7 @@ public static XamlAccessLevel PrivateAccessTo(string assemblyQualifiedTypeName)

public AssemblyName AssemblyAccessToAssemblyName
{
get { return new AssemblyName(AssemblyNameString); }
get => new AssemblyName(AssemblyNameString);
}

public string PrivateAccessToTypeName { get; private set; }
Expand All @@ -92,20 +100,21 @@ internal XamlAccessLevel AssemblyOnly()

internal static XamlAccessLevel FromXml(SecurityElement elem)
{
if (elem.Tag != XmlConstants.XamlAccessLevel)
if (elem.Tag != XamlAccessLevelTagName)
{
throw new ArgumentException(SR.Get(SRID.SecurityXmlUnexpectedTag, elem.Tag, XmlConstants.XamlAccessLevel), nameof(elem));
throw new ArgumentException(SR.Get(SRID.SecurityXmlUnexpectedTag, elem.Tag, XamlAccessLevelTagName), nameof(elem));
}

string assemblyNameString = elem.Attribute(XmlConstants.AssemblyName);
string assemblyNameString = elem.Attribute(AssemblyNameAttributeName);
if (assemblyNameString == null)
{
throw new ArgumentException(SR.Get(SRID.SecurityXmlMissingAttribute, XmlConstants.AssemblyName), nameof(elem));
throw new ArgumentException(SR.Get(SRID.SecurityXmlMissingAttribute, AssemblyNameAttributeName), nameof(elem));
}

AssemblyName assemblyName = new AssemblyName(assemblyNameString);
ValidateAssemblyName(assemblyName, "elem");
ValidateAssemblyName(assemblyName, nameof(elem));

string typeName = elem.Attribute(XmlConstants.TypeName);
string typeName = elem.Attribute(TypeNameAttributeName);
if (typeName != null)
{
typeName = typeName.Trim();
Expand All @@ -116,17 +125,21 @@ internal static XamlAccessLevel FromXml(SecurityElement elem)

internal bool Includes(XamlAccessLevel other)
{
return other.AssemblyNameString == this.AssemblyNameString &&
(other.PrivateAccessToTypeName == null || other.PrivateAccessToTypeName == this.PrivateAccessToTypeName);
if (other.AssemblyNameString != AssemblyNameString)
{
return false;
}

return other.PrivateAccessToTypeName == null || other.PrivateAccessToTypeName == PrivateAccessToTypeName;
}

internal SecurityElement ToXml()
{
SecurityElement element = new SecurityElement(XmlConstants.XamlAccessLevel);
element.AddAttribute(XmlConstants.AssemblyName, AssemblyNameString);
SecurityElement element = new SecurityElement(XamlAccessLevelTagName);
element.AddAttribute(AssemblyNameAttributeName, AssemblyNameString);
if (PrivateAccessToTypeName != null)
{
element.AddAttribute(XmlConstants.TypeName, PrivateAccessToTypeName);
element.AddAttribute(TypeNameAttributeName, PrivateAccessToTypeName);
}
return element;
}
Expand All @@ -139,12 +152,5 @@ private static void ValidateAssemblyName(AssemblyName assemblyName, string argNa
throw new ArgumentException(SR.Get(SRID.ExpectedQualifiedAssemblyName, assemblyName.FullName), argName);
}
}

private static class XmlConstants
{
public const string XamlAccessLevel = "XamlAccessLevel";
public const string AssemblyName = "AssemblyName";
public const string TypeName = "TypeName";
}
}
}
108 changes: 49 additions & 59 deletions src/Microsoft.DotNet.Wpf/src/System.Xaml/System/Xaml/Permissions/XamlLoadPermission.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ public sealed class XamlLoadPermission : CodeAccessPermission, IUnrestrictedPerm
private static IList<XamlAccessLevel> s_emptyAccessLevel;
private bool _isUnrestricted;

private const string IPermissionTagName = "IPermission";
Copy link
Copy Markdown
Member

@danmoseley danmoseley Dec 27, 2018

Choose a reason for hiding this comment

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

Should all this CAS stuff be reduced to stubs, as we don't support CAS on .NET Core?

Copy link
Copy Markdown
Contributor Author

@hughbe hughbe Jan 7, 2019

Choose a reason for hiding this comment

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

Not sure. Probably not something for this PR, opened #241

private const string ClassAttributeName = "class";
private const string VersionAttributeName = "version";
private const string VersionNumber = "1";
private const string UnrestrictedAttributeName = "Unrestricted";

public XamlLoadPermission(PermissionState state)
{
Init(state == PermissionState.Unrestricted, null);
Expand All @@ -28,6 +34,7 @@ public XamlLoadPermission(XamlAccessLevel allowedAccess)
{
throw new ArgumentNullException(nameof(allowedAccess));
}

Init(false, new XamlAccessLevel[] { allowedAccess });
}

Expand All @@ -37,20 +44,21 @@ public XamlLoadPermission(IEnumerable<XamlAccessLevel> allowedAccess)
{
throw new ArgumentNullException(nameof(allowedAccess));
}
List<XamlAccessLevel> accessList = new List<XamlAccessLevel>(allowedAccess);

var accessList = new List<XamlAccessLevel>(allowedAccess);
foreach (XamlAccessLevel accessLevel in allowedAccess)
{
if (accessLevel == null)
{
throw new ArgumentException(SR.Get(SRID.CollectionCannotContainNulls, "allowedAccess"));
throw new ArgumentException(SR.Get(SRID.CollectionCannotContainNulls, nameof(allowedAccess)), nameof(allowedAccess));
}

accessList.Add(accessLevel);
}
Init(false, accessList);
}

#if NETCOREAPP3_0

[ComVisible(false)]
public override bool Equals(object obj)
{
Expand Down Expand Up @@ -84,12 +92,7 @@ public override bool Equals(object obj)
}

[ComVisible(false)]
public override int GetHashCode()
{
// This implementation is only to silence a compiler warning
return base.GetHashCode();
}

public override int GetHashCode() => base.GetHashCode();
#endif

// copy ctor. We can reuse the list of the existing instance, because it is a
Expand All @@ -98,7 +101,7 @@ public override int GetHashCode()
private XamlLoadPermission(XamlLoadPermission other)
{
_isUnrestricted = other._isUnrestricted;
this.AllowedAccess = other.AllowedAccess;
AllowedAccess = other.AllowedAccess;
}

private void Init(bool isUnrestricted, IList<XamlAccessLevel> allowedAccess)
Expand All @@ -121,35 +124,32 @@ private void Init(bool isUnrestricted, IList<XamlAccessLevel> allowedAccess)

public IList<XamlAccessLevel> AllowedAccess { get; private set; } // ReadOnlyCollection

public override IPermission Copy()
{
return new XamlLoadPermission(this);
}
public override IPermission Copy() => new XamlLoadPermission(this);

public override void FromXml(SecurityElement elem)
{
if (elem == null)
{
throw new ArgumentNullException(nameof(elem));
}
if (elem.Tag != XmlConstants.IPermission)
if (elem.Tag != IPermissionTagName)
{
throw new ArgumentException(SR.Get(SRID.SecurityXmlUnexpectedTag, elem.Tag, XmlConstants.IPermission), nameof(elem));
throw new ArgumentException(SR.Get(SRID.SecurityXmlUnexpectedTag, elem.Tag, IPermissionTagName), nameof(elem));
}

string className = elem.Attribute(XmlConstants.Class);
if (!className.StartsWith(GetType().FullName, false, TypeConverterHelper.InvariantEnglishUS))
string className = elem.Attribute(ClassAttributeName);
if (className == null || !className.StartsWith(GetType().FullName, false, TypeConverterHelper.InvariantEnglishUS))
{
throw new ArgumentException(SR.Get(SRID.SecurityXmlUnexpectedValue, className, XmlConstants.Class, GetType().FullName), nameof(elem));
throw new ArgumentException(SR.Get(SRID.SecurityXmlUnexpectedValue, className, ClassAttributeName, GetType().FullName), nameof(elem));
}

string version = elem.Attribute(XmlConstants.Version);
if (version != null && version != XmlConstants.VersionNumber)
string version = elem.Attribute(VersionAttributeName);
if (version != null && version != VersionNumber)
{
throw new ArgumentException(SR.Get(SRID.SecurityXmlUnexpectedValue, className, XmlConstants.Version, XmlConstants.VersionNumber), nameof(elem));
throw new ArgumentException(SR.Get(SRID.SecurityXmlUnexpectedValue, className, VersionAttributeName, VersionNumber), nameof(elem));
}

string unrestricted = elem.Attribute(XmlConstants.Unrestricted);
string unrestricted = elem.Attribute(UnrestrictedAttributeName);
if (unrestricted != null && bool.Parse(unrestricted))
{
Init(true, null);
Expand All @@ -175,6 +175,7 @@ public bool Includes(XamlAccessLevel requestedAccess)
{
throw new ArgumentNullException(nameof(requestedAccess));
}

if (_isUnrestricted)
{
return true;
Expand All @@ -195,20 +196,21 @@ public override IPermission Intersect(IPermission target)
{
return null;
}
XamlLoadPermission other = CastPermission(target, "target");

XamlLoadPermission other = CastPermission(target, nameof(target));
if (other.IsUnrestricted())
{
return this.Copy();
return Copy();
}
if (this.IsUnrestricted())
if (IsUnrestricted())
{
return other.Copy();
}

List<XamlAccessLevel> result = new List<XamlAccessLevel>();
var result = new List<XamlAccessLevel>();
// We could optimize this with a hash, but we don't expect people to be creating
// large unions of access levels.
foreach (XamlAccessLevel accessLevel in this.AllowedAccess)
foreach (XamlAccessLevel accessLevel in AllowedAccess)
{
// First try the full access level
if (other.Includes(accessLevel))
Expand All @@ -232,20 +234,20 @@ public override bool IsSubsetOf(IPermission target)
{
if (target == null)
{
bool isEmpty = !IsUnrestricted() && AllowedAccess.Count == 0;
return isEmpty;
return !IsUnrestricted() && AllowedAccess.Count == 0;
}
XamlLoadPermission other = CastPermission(target, "target");

XamlLoadPermission other = CastPermission(target, nameof(target));
if (other.IsUnrestricted())
{
return true;
}
if (this.IsUnrestricted())
if (IsUnrestricted())
{
return false;
}

foreach (XamlAccessLevel accessLevel in this.AllowedAccess)
foreach (XamlAccessLevel accessLevel in AllowedAccess)
{
if (!other.Includes(accessLevel))
{
Expand All @@ -257,41 +259,42 @@ public override bool IsSubsetOf(IPermission target)

public override SecurityElement ToXml()
{
SecurityElement securityElement = new SecurityElement(XmlConstants.IPermission);
securityElement.AddAttribute(XmlConstants.Class, this.GetType().AssemblyQualifiedName);
securityElement.AddAttribute(XmlConstants.Version, XmlConstants.VersionNumber);
var element = new SecurityElement(IPermissionTagName);
element.AddAttribute(ClassAttributeName, GetType().AssemblyQualifiedName);
element.AddAttribute(VersionAttributeName, VersionNumber);

if (IsUnrestricted())
{
securityElement.AddAttribute(XmlConstants.Unrestricted, Boolean.TrueString);
element.AddAttribute(UnrestrictedAttributeName, Boolean.TrueString);
}
else
{
foreach (XamlAccessLevel accessLevel in AllowedAccess)
{
securityElement.AddChild(accessLevel.ToXml());
element.AddChild(accessLevel.ToXml());
}
}

return securityElement;
return element;
}

public override IPermission Union(IPermission other)
{
if (other == null)
{
return this.Copy();
return Copy();
}
XamlLoadPermission xamlOther = CastPermission(other, "other");

XamlLoadPermission xamlOther = CastPermission(other, nameof(other));
if (IsUnrestricted() || xamlOther.IsUnrestricted())
{
return new XamlLoadPermission(PermissionState.Unrestricted);
}

List<XamlAccessLevel> mergedAccess = new List<XamlAccessLevel>(this.AllowedAccess);
var mergedAccess = new List<XamlAccessLevel>(AllowedAccess);
foreach (XamlAccessLevel accessLevel in xamlOther.AllowedAccess)
{
if (!this.Includes(accessLevel))
if (!Includes(accessLevel))
{
mergedAccess.Add(accessLevel);
if (accessLevel.PrivateAccessToTypeName != null)
Expand All @@ -312,29 +315,16 @@ public override IPermission Union(IPermission other)
return new XamlLoadPermission(mergedAccess);
}

public bool IsUnrestricted()
{
return _isUnrestricted;
}
public bool IsUnrestricted() => _isUnrestricted;

private static XamlLoadPermission CastPermission(IPermission other, string argName)
{
XamlLoadPermission result = other as XamlLoadPermission;
if (result == null)
if (!(other is XamlLoadPermission result))
{
throw new ArgumentException(SR.Get(SRID.ExpectedLoadPermission), argName);
}
return result;
}

private static class XmlConstants
{
public const string IPermission = "IPermission";
public const string Class = "class";
public const string Version = "version";
public const string VersionNumber = "1";
public const string Unrestricted = "Unrestricted";
return result;
}
}

}