Skip to content

Fix write-in-try analyzer for local storage syscalls #1393

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

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Fix write-in-try analyzer for local storage syscalls #1393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4dcf801
Fix write-in-try analyzer for local storage syscalls
Jim8y Sep 23, 2025
6055a52
Ensure temp compilation uses .NET 9
Jim8y Sep 23, 2025
92099c1
Merge branch 'dev' into fix-write-in-try-local-storage
Wi1l-B0t Sep 26, 2025
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
29 changes: 28 additions & 1 deletion src/Neo.Compiler.CSharp/CompilationEngine/CompilationEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
using System.IO;
using System.Linq;
using System.Reflection;
using System.Runtime.Versioning;
using System.Threading.Tasks;
using System.Xml.Linq;
using BigInteger = System.Numerics.BigInteger;
Expand Down Expand Up @@ -146,11 +147,13 @@ public List<CompilationContext> CompileSources(CompilationSourceReferences refer
{string.Join(Environment.NewLine, references.Projects!.Select(u => $" <ProjectReference Include =\"{u}\"/>"))}
</ItemGroup>";

string targetFramework = GetTargetFrameworkMoniker();

var csproj = $@"
<Project Sdk=""Microsoft.NET.Sdk"">

<PropertyGroup>
<TargetFramework>{AppContext.TargetFrameworkName!}</TargetFramework>
<TargetFramework>{targetFramework}</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>
Expand Down Expand Up @@ -216,6 +219,30 @@ public List<CompilationContext> CompileProject(string csproj, List<INamedTypeSym
return targetContractName == null ? CompileProjectContractsWithPrepare(sortedClasses, classDependencies, allClassSymbols) : [CompileProjectContractWithPrepare(sortedClasses, classDependencies, allClassSymbols, targetContractName)];
}

private static string GetTargetFrameworkMoniker()
{
const string fallback = "net9.0";
string? tfm = AppContext.TargetFrameworkName;
if (string.IsNullOrEmpty(tfm))
return fallback;

try
{
FrameworkName framework = new(tfm);
if (!string.Equals(framework.Identifier, ".NETCoreApp", StringComparison.OrdinalIgnoreCase))
return fallback;

if (framework.Version.Major < 9)
return fallback;

return $"net{framework.Version.Major}.{framework.Version.Minor}";
}
catch
{
return fallback;
}
}

public (List<INamedTypeSymbol> sortedClasses, Dictionary<INamedTypeSymbol, List<INamedTypeSymbol>> classDependencies, List<INamedTypeSymbol?> allClassSymbols) PrepareProjectContracts(string csproj)
{
Compilation ??= GetCompilation(csproj);
Expand Down
102 changes: 79 additions & 23 deletions src/Neo.Compiler.CSharp/Optimizer/Analysers/TryCatchFinallyCoverage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ public TryCatchFinallySingleCoverage(ContractInBasicBlocks contractInBasicBlocks

public class TryCatchFinallyCoverage
{
private static readonly bool EnableDiagnostics = Environment.GetEnvironmentVariable("TRY_COVERAGE_DEBUG") == "1";

public ContractInBasicBlocks contractInBasicBlocks { get; protected set; }
// key: start of try block. prevBlock.instructions.Last() is TRY
public Dictionary<BasicBlock, TryCatchFinallySingleCoverage> allTry { get; protected set; }
Expand All @@ -103,7 +105,7 @@ public TryCatchFinallyCoverage(ContractInBasicBlocks contractInBasicBlocks)
{
Stack<(BasicBlock tryBlock, BasicBlock? endFinallyBlock, TryType tryType, bool continueAfterFinally)> tryStack = new();
tryStack.Push((b, null, TryType.TRY, true));
CoverSingleTry(b, tryStack);
ContinueToBlock(b, tryStack);
}
}

Expand All @@ -113,16 +115,17 @@ public BranchType CoverSingleTry(BasicBlock currentBlock,
if (tryStack.Count <= 0)
return BranchType.OK;
tryStack = InstructionCoverage.CopyStack(tryStack);
(BasicBlock tryBlock, BasicBlock? endFinallyBlock, TryType tryType, bool continueAfterFinally) = tryStack.Peek();
HashSet<BasicBlock> handledBlocks = tryType switch
{
TryType.TRY => allTry[tryBlock].tryBlocks,
TryType.CATCH => allTry[tryBlock].catchBlocks,
TryType.FINALLY => allTry[tryBlock].finallyBlocks,
_ => throw new ArgumentException($"Invalid {nameof(tryType)} {tryType}"),
};
while (true)
{
(BasicBlock tryBlock, BasicBlock? endFinallyBlock, TryType tryType, bool continueAfterFinally) = tryStack.Peek();
HashSet<BasicBlock> handledBlocks = tryType switch
{
TryType.TRY => allTry[tryBlock].tryBlocks,
TryType.CATCH => allTry[tryBlock].catchBlocks,
TryType.FINALLY => allTry[tryBlock].finallyBlocks,
_ => throw new ArgumentException($"Invalid {nameof(tryType)} {tryType}"),
};

if (handledBlocks.Contains(currentBlock))
return currentBlock.branchType;
handledBlocks.Add(currentBlock);
Expand All @@ -135,19 +138,19 @@ public BranchType CoverSingleTry(BasicBlock currentBlock,
foreach (int callaTarget in contractInBasicBlocks.coverage.pushaTargets.Keys)
{
BasicBlock callABlock = contractInBasicBlocks.basicBlocksByStartAddr[callaTarget];
CoverSingleTry(callABlock, tryStack);
ContinueToBlock(callABlock, tryStack);
// TODO: if a PUSHA cannot be covered, do not add it as a CALLA target
}
else
{
int callTarget = ComputeJumpTarget(currentBlock.lastAddr, instruction);
BasicBlock calledBlock = contractInBasicBlocks.basicBlocksByStartAddr[callTarget];
CoverSingleTry(calledBlock, tryStack);
ContinueToBlock(calledBlock, tryStack);
}
if (currentBlock.branchType == BranchType.OK)
if (currentBlock.nextBlock != null)
// nextBlock can still be null, when we call a method in try that ABORTs
return CoverSingleTry(currentBlock.nextBlock!, tryStack);
return ContinueToBlock(currentBlock.nextBlock!, tryStack);
return currentBlock.branchType;
}
if (instruction.OpCode == OpCode.RET)
Expand All @@ -156,6 +159,8 @@ public BranchType CoverSingleTry(BasicBlock currentBlock,
{
if (instruction.OpCode == OpCode.TRY || instruction.OpCode == OpCode.TRY_L)
{
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] TRY in block {currentBlock.startAddr}, state={tryType}, stack={FormatStack(tryStack)}");
HashSet<TryCatchFinallySingleCoverage> handledCoverage = tryType switch
{
TryType.TRY => allTry[tryBlock].nestedTrysInTry,
Expand All @@ -165,7 +170,9 @@ public BranchType CoverSingleTry(BasicBlock currentBlock,
};
handledCoverage.Add(allTry[currentBlock.nextBlock!]);
tryStack.Push((currentBlock.nextBlock!, null, TryType.TRY, true));
CoverSingleTry(currentBlock.nextBlock!, tryStack);
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] push TRY block {currentBlock.nextBlock!.startAddr}, stack={FormatStack(tryStack)}");
ContinueToBlock(currentBlock.nextBlock!, tryStack);
}
if (instruction.OpCode == OpCode.THROW)
{
Expand All @@ -179,59 +186,81 @@ public BranchType CoverSingleTry(BasicBlock currentBlock,
if (allTry[prevTryBlock].catchBlock != null)
{
tryStack.Push((prevTryBlock, null, TryType.CATCH, true));
return CoverSingleTry(allTry[prevTryBlock].catchBlock!, tryStack);
return ContinueToBlock(allTry[prevTryBlock].catchBlock!, tryStack);
}
else if (allTry[prevTryBlock].finallyBlock != null)
{
tryStack.Push((prevTryBlock, null, TryType.FINALLY, false));
if (CoverSingleTry(allTry[prevTryBlock].finallyBlock!, tryStack) == BranchType.ABORT)
if (ContinueToBlock(allTry[prevTryBlock].finallyBlock!, tryStack) == BranchType.ABORT)
return BranchType.ABORT;
return BranchType.THROW;
}
}
if (tryType == TryType.CATCH && allTry[tryBlock].finallyBlock != null)
{
tryStack.Push((prevTryBlock, null, TryType.FINALLY, false));
if (CoverSingleTry(allTry[prevTryBlock].finallyBlock!, tryStack) == BranchType.ABORT)
if (ContinueToBlock(allTry[prevTryBlock].finallyBlock!, tryStack) == BranchType.ABORT)
return BranchType.ABORT;
}
return BranchType.THROW;
}
if (instruction.OpCode == OpCode.ENDTRY || instruction.OpCode == OpCode.ENDTRY_L)
{
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] ENDTRY in block {currentBlock.startAddr}, state={tryType}, stack={FormatStack(tryStack)}");
if (tryType != TryType.TRY && tryType != TryType.CATCH)
throw new BadScriptException("No try stack on ENDTRY");
{
if (EnableDiagnostics)
{
Console.Error.WriteLine($"[TryCoverage] Unexpected ENDTRY without TRY/CATCH context. state={tryType}, block={currentBlock.startAddr}, tryBlock={tryBlock.startAddr}");
foreach (var inst in currentBlock.instructions)
Console.Error.WriteLine($" {inst.OpCode}");
}
throw new BadScriptException($"No try stack on ENDTRY (state={tryType}, block={currentBlock.startAddr}, tryBlock={tryBlock.startAddr})");
}
tryStack.Pop(); // pop the ending TRY or CATCH
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] pop after ENDTRY, stack={FormatStack(tryStack)}");
if (tryType == TryType.TRY && allTry[tryBlock].catchBlock != null)
{
tryStack.Push((tryBlock, null, TryType.CATCH, true));
CoverSingleTry(allTry[tryBlock].catchBlock!, tryStack);
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] push CATCH block {allTry[tryBlock].catchBlock!.startAddr}, stack={FormatStack(tryStack)}");
ContinueToBlock(allTry[tryBlock].catchBlock!, tryStack);
tryStack.Pop(); // Pop the CATCH
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] pop CATCH, stack={FormatStack(tryStack)}");
}
int endPointer = ComputeJumpTarget(currentBlock.lastAddr, instruction);
endFinallyBlock = contractInBasicBlocks.basicBlocksByStartAddr[endPointer];
BasicBlock nextBlock;
if (allTry[tryBlock].finallyBlock != null)
{
tryStack.Push(new(tryBlock, endFinallyBlock, TryType.FINALLY, true));
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] push FINALLY block {allTry[tryBlock].finallyBlock!.startAddr}, end={endFinallyBlock.startAddr}, stack={FormatStack(tryStack)}");
nextBlock = allTry[tryBlock].finallyBlock!;
}
else
{
allTry[tryBlock].endingBlocks.Add(endFinallyBlock);
nextBlock = endFinallyBlock;
}
return CoverSingleTry(nextBlock, tryStack);
return ContinueToBlock(nextBlock, tryStack);
}
if (instruction.OpCode == OpCode.ENDFINALLY)
{
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] ENDFINALLY in block {currentBlock.startAddr}, state={tryType}, stack={FormatStack(tryStack)}");
if (tryType != TryType.FINALLY)
throw new BadScriptException("No finally stack on ENDFINALLY");
tryStack.Pop(); // pop the ending FINALLY
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] pop FINALLY, stack={FormatStack(tryStack)}");
if (continueAfterFinally)
{
allTry[tryBlock].endingBlocks.Add(endFinallyBlock!);
return CoverSingleTry(endFinallyBlock!, tryStack);
return ContinueToBlock(endFinallyBlock!, tryStack);
}
// For this basic block in finally, the branch type is OK
// The throw is caused by previous codes
Expand All @@ -242,14 +271,14 @@ public BranchType CoverSingleTry(BasicBlock currentBlock,
{
int target = ComputeJumpTarget(currentBlock.lastAddr, instruction);
BasicBlock targetBlock = contractInBasicBlocks.basicBlocksByStartAddr[target];
return CoverSingleTry(targetBlock, tryStack);
return ContinueToBlock(targetBlock, tryStack);
}
if (conditionalJump.Contains(instruction.OpCode) || conditionalJump_L.Contains(instruction.OpCode))
{
BranchType noJump = CoverSingleTry(currentBlock.nextBlock!, tryStack);
BranchType noJump = ContinueToBlock(currentBlock.nextBlock!, tryStack);
int target = ComputeJumpTarget(currentBlock.lastAddr, instruction);
BasicBlock targetBlock = contractInBasicBlocks.basicBlocksByStartAddr[target];
BranchType jump = CoverSingleTry(targetBlock, tryStack);
BranchType jump = ContinueToBlock(targetBlock, tryStack);
if (noJump == BranchType.OK || jump == BranchType.OK)
return BranchType.OK;
if (noJump == BranchType.ABORT && jump == BranchType.ABORT)
Expand All @@ -260,5 +289,32 @@ public BranchType CoverSingleTry(BasicBlock currentBlock,
currentBlock = currentBlock.nextBlock!;
}
}

private static string FormatStack(Stack<(BasicBlock tryBlock, BasicBlock? endFinallyBlock, TryType tryType, bool continueAfterFinally)> stack)
=> string.Join(" | ", stack.Select(s => $"[{s.tryBlock.startAddr}:{s.tryType}:{(s.endFinallyBlock?.startAddr.ToString() ?? "-")}:cont={s.continueAfterFinally}]"));

private BranchType ContinueToBlock(
BasicBlock targetBlock,
Stack<(BasicBlock tryBlock, BasicBlock? endFinallyBlock, TryType tryType, bool continueAfterFinally)> tryStack)
{
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] Continue to block {targetBlock.startAddr}, stack={FormatStack(tryStack)}");
if (tryStack.Count > 0)
{
(BasicBlock tryBlock, BasicBlock? endFinallyBlock, TryType tryType, bool continueAfterFinally) = tryStack.Peek();
if (tryType == TryType.FINALLY && endFinallyBlock != null && ReferenceEquals(targetBlock, endFinallyBlock))
{
Stack<(BasicBlock tryBlock, BasicBlock? endFinallyBlock, TryType tryType, bool continueAfterFinally)> stackAfterFinally = InstructionCoverage.CopyStack(tryStack);
stackAfterFinally.Pop();
if (EnableDiagnostics)
Console.Error.WriteLine($"[TryCoverage] transition after finally end, stack={FormatStack(stackAfterFinally)}");
if (!continueAfterFinally)
return BranchType.OK;
allTry[tryBlock].endingBlocks.Add(endFinallyBlock);
return CoverSingleTry(targetBlock, stackAfterFinally);
}
}
return CoverSingleTry(targetBlock, tryStack);
}
}
}
14 changes: 10 additions & 4 deletions src/Neo.Compiler.CSharp/SecurityAnalyzer/WriteInTryAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ namespace Neo.Compiler.SecurityAnalyzer
{
public static class WriteInTryAnalzyer
{
private static readonly HashSet<uint> StorageWriteHashes = new()
{
ApplicationEngine.System_Storage_Put.Hash,
ApplicationEngine.System_Storage_Delete.Hash,
ApplicationEngine.System_Storage_Local_Put.Hash,
ApplicationEngine.System_Storage_Local_Delete.Hash
};

public class WriteInTryVulnerability
{
// key block writes storage; value blocks in try
Expand Down Expand Up @@ -63,8 +71,7 @@ public string GetWarningInfo(bool print = false)
foreach (VM.Instruction i in b.instructions)
{
if (i.OpCode == VM.OpCode.SYSCALL
&& (i.TokenU32 == ApplicationEngine.System_Storage_Put.Hash
|| i.TokenU32 == ApplicationEngine.System_Storage_Delete.Hash))
&& StorageWriteHashes.Contains(i.TokenU32))
writeAddrs.Add(a);
a += i.Size;
}
Expand Down Expand Up @@ -133,8 +140,7 @@ public static WriteInTryVulnerability AnalyzeWriteInTry
foreach (BasicBlock block in contractInBasicBlocks.sortedBasicBlocks)
foreach (VM.Instruction i in block.instructions)
if (i.OpCode == VM.OpCode.SYSCALL
&& (i.TokenU32 == ApplicationEngine.System_Storage_Put.Hash
|| i.TokenU32 == ApplicationEngine.System_Storage_Delete.Hash))
&& StorageWriteHashes.Contains(i.TokenU32))
allBasicBlocksWritingStorage.Add(block);
foreach (TryCatchFinallySingleCoverage c in tryCatchFinallyCoverage.allTry.Values)
{
Expand Down
Loading