Skip to content

CheckWitnessAnalyzer #1270

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.

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
merged 3 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
71 changes: 71 additions & 0 deletions src/Neo.Compiler.CSharp/SecurityAnalyzer/CheckWitnessAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
using Neo.Json;
using Neo.Optimizer;
using Neo.SmartContract;
using Neo.SmartContract.Manifest;
using Neo.SmartContract.Native;
using Neo.VM;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Neo.Compiler.SecurityAnalyzer
{
/// <summary>
/// Always Assert(CheckWitness(someone))
/// Do not just CheckWitness(someone)
/// </summary>
public static class CheckWitnessAnalyzer
{
public class CheckWitnessVulnerability
{
public readonly List<int> droppedCheckWitnessResults;
public readonly JToken? debugInfo;
public CheckWitnessVulnerability(
List<int> droppedCheckWitnessResults,
JToken? debugInfo = null)
{
this.droppedCheckWitnessResults = droppedCheckWitnessResults;
this.debugInfo = debugInfo;
}

public string GetWarningInfo(bool print = false)
{
if (droppedCheckWitnessResults.Count == 0)
return "";
string result = $"[SEC] The returned values of CheckWitness at the following instruction addresses are DROPped:{Environment.NewLine}" +
Copy link
Contributor

Choose a reason for hiding this comment

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

DROPped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DROP after SYSCALL F8-27-EC-8C # System.Runtime.CheckWitness SysCall

Copy link
Contributor

Choose a reason for hiding this comment

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

this is a typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am emphasizing the instruction DROP. Can change if not lokking good.

$"\t{string.Join(", ", droppedCheckWitnessResults)}{Environment.NewLine}" +
$"You should typically `Assert(CheckWitness({nameof(UInt160)} someone))`{Environment.NewLine}" +
$"instead of just `CheckWitness({nameof(UInt160)} someone)`{Environment.NewLine}";
if (print)
Console.Write(result);
return result;
}
}

/// <summary>
///
/// </summary>
/// <param name="nef"></param>
/// <param name="manifest"></param>
/// <param name="debugInfo"></param>
Comment on lines +47 to +51
Copy link
Member

Choose a reason for hiding this comment

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

This should be filled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be filled

On it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

public static CheckWitnessVulnerability AnalyzeCheckWitness
(NefFile nef, ContractManifest manifest, JToken? debugInfo = null)
{
(int addr, VM.Instruction instruction)[] instructions = ((Script)nef.Script).EnumerateInstructions().ToArray();
List<int> result = [];
for (int i = 0; i < instructions.Length; ++i)
{
VM.Instruction instruction = instructions[i].instruction;
if (instruction.OpCode == OpCode.SYSCALL && instruction.TokenU32 == ApplicationEngine.System_Runtime_CheckWitness.Hash)
{
if (i + 1 >= instructions.Length)
return new CheckWitnessVulnerability(result);
if (instructions[i + 1].instruction.OpCode == OpCode.DROP)
result.Add(i);
}
}
return new CheckWitnessVulnerability(result);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public static void AnalyzeWithPrint(NefFile nef, ContractManifest manifest, JTok
{
ReEntrancyAnalyzer.AnalyzeSingleContractReEntrancy(nef, manifest, debugInfo).GetWarningInfo(print: true);
WriteInTryAnalzyer.AnalyzeWriteInTry(nef, manifest, debugInfo).GetWarningInfo(print: true);
CheckWitnessAnalyzer.AnalyzeCheckWitness(nef, manifest, debugInfo).GetWarningInfo(print: true);
if (!UpdateAnalzyer.AnalyzeUpdate(nef, manifest, debugInfo))
Console.WriteLine("[SEC] This contract cannot be updated, or maybe you used abstract code styles to update it.");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
using Neo.SmartContract.Framework;
using Neo.SmartContract.Framework.Attributes;
using Neo.SmartContract.Framework.Native;
using Neo.SmartContract.Framework.Services;

namespace Neo.Compiler.CSharp.TestContracts
{
#pragma warning disable CS8625
public class Contract_CheckWitness : SmartContract.Framework.SmartContract
{
public static void Main(UInt160 u)

Check warning on line 11 in tests/Neo.Compiler.CSharp.TestContracts/Contract_SecurityAnalyzer/Contract_CheckWitness.cs

View workflow job for this annotation

GitHub Actions / Test

'Contract_CheckWitness.Main(UInt160)' has the wrong signature to be an entry point
{
Runtime.CheckWitness(u);
ExecutionEngine.Assert(Runtime.CheckWitness(u));
}
#pragma warning restore CS8625
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Neo.Compiler.SecurityAnalyzer;
using Neo.Optimizer;
using Neo.SmartContract.Testing;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Neo.Compiler.CSharp.UnitTests.SecurityAnalyzer
{
[TestClass]
public class CheckWitnessTests : DebugAndTestBase<Contract_CheckWitness>
{
[TestMethod]
public void Test_CheckWitness()
{
CheckWitnessAnalyzer.CheckWitnessVulnerability result = CheckWitnessAnalyzer.AnalyzeCheckWitness(NefFile, Manifest, null);
Assert.AreEqual(result.droppedCheckWitnessResults.Count, 1);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using Neo.Cryptography.ECC;
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Numerics;

namespace Neo.SmartContract.Testing;

public abstract class Contract_CheckWitness(Neo.SmartContract.Testing.SmartContractInitialize initialize) : Neo.SmartContract.Testing.SmartContract(initialize), IContractInfo
{
#region Compiled data

public static Neo.SmartContract.Manifest.ContractManifest Manifest => Neo.SmartContract.Manifest.ContractManifest.Parse(@"{""name"":""Contract_CheckWitness"",""groups"":[],""features"":{},""supportedstandards"":[],""abi"":{""methods"":[{""name"":""main"",""parameters"":[{""name"":""u"",""type"":""Hash160""}],""returntype"":""Void"",""offset"":0,""safe"":false}],""events"":[]},""permissions"":[],""trusts"":[],""extra"":{""nef"":{""optimization"":""All""}}}");

/// <summary>
/// Optimization: "All"
/// </summary>
public static Neo.SmartContract.NefFile Nef => Neo.IO.Helper.AsSerializable<Neo.SmartContract.NefFile>(Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABJXAAF4Qfgn7IxFeEH4J+yMOUD4hGLi"));

#endregion

#region Unsafe methods

/// <summary>
/// Unsafe method
/// </summary>
/// <remarks>
/// Script: VwABeEH4J+yMRXhB+CfsjDlA
/// INITSLOT 0001 [64 datoshi]
/// LDARG0 [2 datoshi]
/// SYSCALL F827EC8C 'System.Runtime.CheckWitness' [1024 datoshi]
/// DROP [2 datoshi]
/// LDARG0 [2 datoshi]
/// SYSCALL F827EC8C 'System.Runtime.CheckWitness' [1024 datoshi]
/// ASSERT [1 datoshi]
/// RET [0 datoshi]
/// </remarks>
[DisplayName("main")]
public abstract void Main(UInt160? u);

#endregion
}
Loading