Try to report method source of sandboxing issues.

This commit is contained in:
Pieter-Jan Briers
2024-06-20 21:41:08 +02:00
parent 738cfbe992
commit 87bb29408a
3 changed files with 480 additions and 24 deletions

View File

@@ -40,6 +40,7 @@ END TEMPLATE-->
### New features
* `System.Collections.IList` and `System.Collections.ICollection` are now sandbox safe, this fixes some collection expression cases.
* The sandboxing system will now report the methods responsible for references to illegal items.
### Bugfixes

View File

@@ -0,0 +1,442 @@
#if TOOLS
using System;
using System.Buffers.Binary;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection.Metadata;
using System.Reflection.PortableExecutable;
using System.Runtime.CompilerServices;
namespace Robust.Shared.ContentPack;
internal sealed partial class AssemblyTypeChecker
{
// This part of the code tries to find the originator of bad sandbox references.
private void ReportBadReferences(PEReader peReader, MetadataReader reader, IEnumerable<EntityHandle> reference)
{
_sawmill.Info("Started search for originator of bad references...");
var refs = reference.ToHashSet();
ExpandReferences(reader, refs);
foreach (var methodDefHandle in reader.MethodDefinitions)
{
var methodDef = reader.GetMethodDefinition(methodDefHandle);
if (methodDef.RelativeVirtualAddress == 0)
continue;
var methodName = reader.GetString(methodDef.Name);
var body = peReader.GetMethodBody(methodDef.RelativeVirtualAddress);
var bytes = body.GetILBytes()!;
var ilReader = new ILReader(bytes);
var prefPosition = 0;
while (ilReader.MoveNext(out var instruction))
{
if (instruction.TryGetEntityHandle(out var handle))
{
if (refs.Contains(handle))
{
var type = GetTypeFromDefinition(reader, methodDef.GetDeclaringType());
_sawmill.Error(
$"Found reference to {DisplayHandle(reader, handle)} in method {type}.{methodName} at IL 0x{prefPosition:X4}");
}
}
prefPosition = ilReader.Position;
}
}
}
private static string DisplayHandle(MetadataReader reader, EntityHandle handle)
{
switch (handle.Kind)
{
case HandleKind.MemberReference:
var memberRef = reader.GetMemberReference((MemberReferenceHandle)handle);
var name = reader.GetString(memberRef.Name);
var parent = DisplayHandle(reader, memberRef.Parent);
return $"{parent}.{name}";
case HandleKind.TypeReference:
return $"{ParseTypeReference(reader, (TypeReferenceHandle)handle)}";
case HandleKind.TypeSpecification:
var typeSpec = reader.GetTypeSpecification((TypeSpecificationHandle)handle);
var provider = new TypeProvider();
var type = typeSpec.DecodeSignature(provider, 0);
return $"{type}";
default:
return $"({handle.Kind} handle)";
}
}
private static void ExpandReferences(MetadataReader reader, HashSet<EntityHandle> handles)
{
var toAdd = new List<EntityHandle>();
foreach (var memberRefHandle in reader.MemberReferences)
{
var memberRef = reader.GetMemberReference(memberRefHandle);
if (handles.Contains(memberRef.Parent))
{
toAdd.Add(memberRefHandle);
}
}
handles.UnionWith(toAdd);
}
private readonly struct ILInstruction
{
public readonly ILOpCode OpCode;
public readonly long Argument;
public readonly int[]? SwitchTargets;
public ILInstruction(ILOpCode opCode)
{
OpCode = opCode;
}
public ILInstruction(ILOpCode opCode, long argument)
{
OpCode = opCode;
Argument = argument;
}
public ILInstruction(ILOpCode opCode, long argument, int[] switchTargets)
{
OpCode = opCode;
Argument = argument;
SwitchTargets = switchTargets;
}
public bool TryGetEntityHandle(out EntityHandle handle)
{
switch (OpCode)
{
case ILOpCode.Call:
case ILOpCode.Callvirt:
case ILOpCode.Newobj:
case ILOpCode.Jmp:
case ILOpCode.Box:
case ILOpCode.Castclass:
case ILOpCode.Cpobj:
case ILOpCode.Initobj:
case ILOpCode.Isinst:
case ILOpCode.Ldelem:
case ILOpCode.Ldelema:
case ILOpCode.Ldfld:
case ILOpCode.Ldflda:
case ILOpCode.Ldobj:
case ILOpCode.Ldstr:
case ILOpCode.Ldtoken:
case ILOpCode.Ldvirtftn:
case ILOpCode.Mkrefany:
case ILOpCode.Newarr:
case ILOpCode.Refanyval:
case ILOpCode.Sizeof:
case ILOpCode.Stelem:
case ILOpCode.Stfld:
case ILOpCode.Stobj:
case ILOpCode.Stsfld:
case ILOpCode.Throw:
case ILOpCode.Unbox_any:
handle = Unsafe.BitCast<int, EntityHandle>((int)Argument);
return true;
default:
handle = default;
return false;
}
}
}
private sealed class ILReader(byte[] body)
{
public int Position;
public bool MoveNext(out ILInstruction instruction)
{
if (Position >= body.Length)
{
instruction = default;
return false;
}
var firstByte = body[Position++];
var opCode = (ILOpCode)firstByte;
if (firstByte == 0xFE)
opCode = 0xFE00 + (ILOpCode)body[Position++];
switch (opCode)
{
// no args.
case ILOpCode.Readonly:
case ILOpCode.Tail:
case ILOpCode.Volatile:
case ILOpCode.Add:
case ILOpCode.Add_ovf:
case ILOpCode.Add_ovf_un:
case ILOpCode.And:
case ILOpCode.Arglist:
case ILOpCode.Break:
case ILOpCode.Ceq:
case ILOpCode.Cgt:
case ILOpCode.Cgt_un:
case ILOpCode.Ckfinite:
case ILOpCode.Clt:
case ILOpCode.Clt_un:
case ILOpCode.Conv_i1:
case ILOpCode.Conv_i2:
case ILOpCode.Conv_i4:
case ILOpCode.Conv_i8:
case ILOpCode.Conv_r4:
case ILOpCode.Conv_r8:
case ILOpCode.Conv_u1:
case ILOpCode.Conv_u2:
case ILOpCode.Conv_u4:
case ILOpCode.Conv_u8:
case ILOpCode.Conv_i:
case ILOpCode.Conv_u:
case ILOpCode.Conv_r_un:
case ILOpCode.Conv_ovf_i1:
case ILOpCode.Conv_ovf_i2:
case ILOpCode.Conv_ovf_i4:
case ILOpCode.Conv_ovf_i8:
case ILOpCode.Conv_ovf_u4:
case ILOpCode.Conv_ovf_u8:
case ILOpCode.Conv_ovf_i:
case ILOpCode.Conv_ovf_u:
case ILOpCode.Conv_ovf_i1_un:
case ILOpCode.Conv_ovf_i2_un:
case ILOpCode.Conv_ovf_i4_un:
case ILOpCode.Conv_ovf_i8_un:
case ILOpCode.Conv_ovf_u4_un:
case ILOpCode.Conv_ovf_u8_un:
case ILOpCode.Conv_ovf_i_un:
case ILOpCode.Conv_ovf_u_un:
case ILOpCode.Cpblk:
case ILOpCode.Div:
case ILOpCode.Div_un:
case ILOpCode.Dup:
case ILOpCode.Endfilter:
case ILOpCode.Endfinally:
case ILOpCode.Initblk:
case ILOpCode.Ldarg_0:
case ILOpCode.Ldarg_1:
case ILOpCode.Ldarg_2:
case ILOpCode.Ldarg_3:
case ILOpCode.Ldc_i4_0:
case ILOpCode.Ldc_i4_1:
case ILOpCode.Ldc_i4_2:
case ILOpCode.Ldc_i4_3:
case ILOpCode.Ldc_i4_4:
case ILOpCode.Ldc_i4_5:
case ILOpCode.Ldc_i4_6:
case ILOpCode.Ldc_i4_7:
case ILOpCode.Ldc_i4_8:
case ILOpCode.Ldc_i4_m1:
case ILOpCode.Ldind_i1:
case ILOpCode.Ldind_u1:
case ILOpCode.Ldind_i2:
case ILOpCode.Ldind_u2:
case ILOpCode.Ldind_i4:
case ILOpCode.Ldind_u4:
case ILOpCode.Ldind_i8:
case ILOpCode.Ldind_i:
case ILOpCode.Ldind_r4:
case ILOpCode.Ldind_r8:
case ILOpCode.Ldind_ref:
case ILOpCode.Ldloc_0:
case ILOpCode.Ldloc_1:
case ILOpCode.Ldloc_2:
case ILOpCode.Ldloc_3:
case ILOpCode.Ldnull:
case ILOpCode.Localloc:
case ILOpCode.Mul:
case ILOpCode.Mul_ovf:
case ILOpCode.Mul_ovf_un:
case ILOpCode.Neg:
case ILOpCode.Nop:
case ILOpCode.Not:
case ILOpCode.Or:
case ILOpCode.Pop:
case ILOpCode.Rem:
case ILOpCode.Rem_un:
case ILOpCode.Ret:
case ILOpCode.Shl:
case ILOpCode.Shr:
case ILOpCode.Shr_un:
case ILOpCode.Stind_i1:
case ILOpCode.Stind_i2:
case ILOpCode.Stind_i4:
case ILOpCode.Stind_i8:
case ILOpCode.Stind_r4:
case ILOpCode.Stind_r8:
case ILOpCode.Stind_i:
case ILOpCode.Stind_ref:
case ILOpCode.Stloc_0:
case ILOpCode.Stloc_1:
case ILOpCode.Stloc_2:
case ILOpCode.Stloc_3:
case ILOpCode.Sub:
case ILOpCode.Sub_ovf:
case ILOpCode.Sub_ovf_un:
case ILOpCode.Xor:
case ILOpCode.Ldelem_i1:
case ILOpCode.Ldelem_u1:
case ILOpCode.Ldelem_i2:
case ILOpCode.Ldelem_u2:
case ILOpCode.Ldelem_i4:
case ILOpCode.Ldelem_u4:
case ILOpCode.Ldelem_i8:
case ILOpCode.Ldelem_i:
case ILOpCode.Ldelem_r4:
case ILOpCode.Ldelem_r8:
case ILOpCode.Ldelem_ref:
case ILOpCode.Ldlen:
case ILOpCode.Refanytype:
case ILOpCode.Rethrow:
case ILOpCode.Stelem_i1:
case ILOpCode.Stelem_i2:
case ILOpCode.Stelem_i4:
case ILOpCode.Stelem_i8:
case ILOpCode.Stelem_i:
case ILOpCode.Stelem_r4:
case ILOpCode.Stelem_r8:
case ILOpCode.Stelem_ref:
case ILOpCode.Throw:
instruction = new ILInstruction(opCode);
break;
// 1-byte arg.
case ILOpCode.Unaligned:
case ILOpCode.Beq_s:
case ILOpCode.Bge_s:
case ILOpCode.Bge_un_s:
case ILOpCode.Bgt_s:
case ILOpCode.Bgt_un_s:
case ILOpCode.Ble_s:
case ILOpCode.Ble_un_s:
case ILOpCode.Blt_s:
case ILOpCode.Blt_un_s:
case ILOpCode.Bne_un_s:
case ILOpCode.Br_s:
case ILOpCode.Brfalse_s:
case ILOpCode.Brtrue_s:
case ILOpCode.Ldarg_s:
case ILOpCode.Ldarga_s:
case ILOpCode.Ldc_i4_s:
case ILOpCode.Ldloc_s:
case ILOpCode.Ldloca_s:
case ILOpCode.Leave_s:
case ILOpCode.Starg_s:
case ILOpCode.Stloc_s:
instruction = new ILInstruction(opCode, body[Position]);
Position += 1;
break;
// 2-byte value
case ILOpCode.Ldarg:
case ILOpCode.Ldarga:
case ILOpCode.Ldloc:
case ILOpCode.Ldloca:
case ILOpCode.Starg:
case ILOpCode.Stloc:
var shortValue = BinaryPrimitives.ReadInt16LittleEndian(body.AsSpan(Position, 2));
Position += 2;
instruction = new ILInstruction(opCode, shortValue);
break;
// 4-byte value
case ILOpCode.Constrained:
case ILOpCode.Beq:
case ILOpCode.Bge:
case ILOpCode.Bge_un:
case ILOpCode.Bgt:
case ILOpCode.Bgt_un:
case ILOpCode.Ble:
case ILOpCode.Ble_un:
case ILOpCode.Blt:
case ILOpCode.Blt_un:
case ILOpCode.Bne_un:
case ILOpCode.Br:
case ILOpCode.Brfalse:
case ILOpCode.Brtrue:
case ILOpCode.Call:
case ILOpCode.Calli:
case ILOpCode.Jmp:
case ILOpCode.Ldc_i4:
case ILOpCode.Ldc_r4:
case ILOpCode.Ldftn:
case ILOpCode.Leave:
case ILOpCode.Box:
case ILOpCode.Callvirt:
case ILOpCode.Castclass:
case ILOpCode.Cpobj:
case ILOpCode.Initobj:
case ILOpCode.Isinst:
case ILOpCode.Ldelem:
case ILOpCode.Ldelema:
case ILOpCode.Ldfld:
case ILOpCode.Ldflda:
case ILOpCode.Ldobj:
case ILOpCode.Ldsfld:
case ILOpCode.Ldsflda:
case ILOpCode.Ldstr:
case ILOpCode.Ldtoken:
case ILOpCode.Ldvirtftn:
case ILOpCode.Mkrefany:
case ILOpCode.Newarr:
case ILOpCode.Newobj:
case ILOpCode.Refanyval:
case ILOpCode.Sizeof:
case ILOpCode.Stelem:
case ILOpCode.Stfld:
case ILOpCode.Stobj:
case ILOpCode.Stsfld:
case ILOpCode.Unbox:
case ILOpCode.Unbox_any:
var intValue = BinaryPrimitives.ReadInt32LittleEndian(body.AsSpan(Position, 4));
Position += 4;
instruction = new ILInstruction(opCode, intValue);
break;
// 8-byte value
case ILOpCode.Ldc_i8:
case ILOpCode.Ldc_r8:
var longValue = BinaryPrimitives.ReadInt64LittleEndian(body.AsSpan(Position, 8));
Position += 8;
instruction = new ILInstruction(opCode, longValue);
break;
// Switch
case ILOpCode.Switch:
var switchLength = BinaryPrimitives.ReadInt32LittleEndian(body.AsSpan(Position, 4));
Position += 4;
var switchArgs = new int[switchLength];
for (var i = 0; i < switchLength; i++)
{
switchArgs[i] = BinaryPrimitives.ReadInt32LittleEndian(body.AsSpan(Position, 4));
Position += 4;
}
instruction = new ILInstruction(opCode, switchLength, switchArgs);
break;
default:
throw new InvalidDataException($"Unknown opcode: {opCode}");
}
return true;
}
}
}
#endif

View File

@@ -148,7 +148,7 @@ namespace Robust.Shared.ContentPack
if ((Dump & DumpFlags.Types) != 0)
{
foreach (var mType in types)
foreach (var (_, mType) in types)
{
_sawmill.Debug($"RefType: {mType}");
}
@@ -156,7 +156,7 @@ namespace Robust.Shared.ContentPack
if ((Dump & DumpFlags.Members) != 0)
{
foreach (var memberRef in members)
foreach (var (_, memberRef) in members)
{
_sawmill.Debug($"RefMember: {memberRef}");
}
@@ -183,14 +183,17 @@ namespace Robust.Shared.ContentPack
var loadedConfig = _config.Result;
#pragma warning restore RA0004
var badRefs = new ConcurrentBag<EntityHandle>();
// We still do explicit type reference scanning, even though the actual whitelists work with raw members.
// This is so that we can simplify handling of generic type specifications during member checking:
// we won't have to check that any types in their type arguments are whitelisted.
foreach (var type in types)
foreach (var (handle, type) in types)
{
if (!IsTypeAccessAllowed(loadedConfig, type, out _))
{
errors.Add(new SandboxError($"Access to type not allowed: {type}"));
badRefs.Add(handle);
}
}
@@ -208,13 +211,20 @@ namespace Robust.Shared.ContentPack
_sawmill.Debug($"Type abuse... {fullStopwatch.ElapsedMilliseconds}ms");
CheckMemberReferences(loadedConfig, members, errors);
CheckMemberReferences(loadedConfig, members, errors, badRefs);
foreach (var error in errors)
{
_sawmill.Error($"Sandbox violation: {error.Message}");
}
#if TOOLS
if (!badRefs.IsEmpty)
{
ReportBadReferences(peReader, reader, badRefs);
}
#endif
_sawmill.Debug($"Checked assembly in {fullStopwatch.ElapsedMilliseconds}ms");
return errors.IsEmpty;
@@ -351,11 +361,13 @@ namespace Robust.Shared.ContentPack
private void CheckMemberReferences(
SandboxConfig sandboxConfig,
List<MMemberRef> members,
ConcurrentBag<SandboxError> errors)
List<(MemberReferenceHandle handle, MMemberRef parsed)> members,
ConcurrentBag<SandboxError> errors,
ConcurrentBag<EntityHandle> badReferences)
{
Parallel.ForEach(members, memberRef =>
Parallel.ForEach(members, entry =>
{
var (handle, memberRef) = entry;
MType baseType = memberRef.ParentType;
while (!(baseType is MTypeReferenced))
{
@@ -416,6 +428,7 @@ namespace Robust.Shared.ContentPack
}
errors.Add(new SandboxError($"Access to field not allowed: {mMemberRefField}"));
badReferences.Add(handle);
break;
}
case MMemberRefMethod mMemberRefMethod:
@@ -444,6 +457,7 @@ namespace Robust.Shared.ContentPack
}
errors.Add(new SandboxError($"Access to method not allowed: {mMemberRefMethod}"));
badReferences.Add(handle);
break;
default:
throw new ArgumentOutOfRangeException(nameof(memberRef));
@@ -458,18 +472,18 @@ namespace Robust.Shared.ContentPack
{
// This inheritance whitelisting primarily serves to avoid content doing funny stuff
// by e.g. inheriting Type.
foreach (var (_, baseType, interfaces) in inherited)
foreach (var (type, baseType, interfaces) in inherited)
{
if (!CanInherit(baseType))
{
errors.Add(new SandboxError($"Inheriting of type not allowed: {baseType}"));
errors.Add(new SandboxError($"Inheriting of type not allowed: {baseType} (by {type})"));
}
foreach (var @interface in interfaces)
{
if (!CanInherit(@interface))
{
errors.Add(new SandboxError($"Implementing of interface not allowed: {@interface}"));
errors.Add(new SandboxError($"Implementing of interface not allowed: {@interface} (by {type})"));
}
}
@@ -547,25 +561,25 @@ namespace Robust.Shared.ContentPack
return nsDict.TryGetValue(type.Name, out cfg);
}
private List<MTypeReferenced> GetReferencedTypes(MetadataReader reader, ConcurrentBag<SandboxError> errors)
private List<(TypeReferenceHandle handle, MTypeReferenced parsed)> GetReferencedTypes(MetadataReader reader, ConcurrentBag<SandboxError> errors)
{
return reader.TypeReferences.Select(typeRefHandle =>
{
try
{
return ParseTypeReference(reader, typeRefHandle);
return (typeRefHandle, ParseTypeReference(reader, typeRefHandle));
}
catch (UnsupportedMetadataException e)
{
errors.Add(new SandboxError(e));
return null;
return default;
}
})
.Where(p => p != null)
.Where(p => p.Item2 != null)
.ToList()!;
}
private List<MMemberRef> GetReferencedMembers(MetadataReader reader, ConcurrentBag<SandboxError> errors)
private List<(MemberReferenceHandle handle, MMemberRef parsed)> GetReferencedMembers(MetadataReader reader, ConcurrentBag<SandboxError> errors)
{
return reader.MemberReferences.AsParallel()
.Select(memRefHandle =>
@@ -586,7 +600,7 @@ namespace Robust.Shared.ContentPack
catch (UnsupportedMetadataException u)
{
errors.Add(new SandboxError(u));
return null;
return default;
}
break;
@@ -600,7 +614,7 @@ namespace Robust.Shared.ContentPack
catch (UnsupportedMetadataException u)
{
errors.Add(new SandboxError(u));
return null;
return default;
}
break;
@@ -616,7 +630,7 @@ namespace Robust.Shared.ContentPack
{
// Ensure this isn't a self-defined type.
// This can happen due to generics since MethodSpec needs to point to MemberRef.
return null;
return default;
}
break;
@@ -625,18 +639,18 @@ namespace Robust.Shared.ContentPack
{
errors.Add(new SandboxError(
$"Module global variables and methods are unsupported. Name: {memName}"));
return null;
return default;
}
case HandleKind.MethodDefinition:
{
errors.Add(new SandboxError($"Vararg calls are unsupported. Name: {memName}"));
return null;
return default;
}
default:
{
errors.Add(new SandboxError(
$"Unsupported member ref parent type: {memRef.Parent.Kind}. Name: {memName}"));
return null;
return default;
}
}
@@ -667,9 +681,9 @@ namespace Robust.Shared.ContentPack
throw new ArgumentOutOfRangeException();
}
return memberRef;
return (memRefHandle, memberRef);
})
.Where(p => p != null)
.Where(p => p.memberRef != null)
.ToList()!;
}
@@ -780,7 +794,6 @@ namespace Robust.Shared.ContentPack
}
}
/// <exception href="UnsupportedMetadataException">
/// Thrown if the metadata does something funny we don't "support" like type forwarding.
/// </exception>