Today's anonymous submitter, who we'll call Sally, works on medical devices. As you can imagine, the development process for such devices is very strict. I mean, it should be, but we know that the industry has problems.
Unfortunately for Sally, one of those problems is the tech lead on a project she is inheriting. Said tech lead is leaving, and Sally is coming on to replace them. The project is in C#, and Sally is the most experienced with the language, making her the obvious choice to step in.
Now, the current tech lead had some concerns about the development cycle. You see, the whole process of writing code, compiling code, deploying that code onto hardware, and then running the code just took too darn long. If you wanted to iterate as fast as possible, you needed to skip some of those steps.
internal static Action<InstrumentState> Compile(IEnumerable<Configuration.Rule> rules)
{
var code = string.Format(@"
using System;
using SomeCompany.SomeProject.Instrument;
using SomeCompany.SomeProject.Instrument.State.Actions;
using ConsoleState = StateMachine.Types.State;
namespace SomeCompany.SomeProject.Instrument.State.Reducers {{
public class Script {{
private static bool _done;
private static void Done() {{ _done = true; }}
public static void Execute(InstrumentState state) {{
_done = false;
{0}
}}
{1}
}}
}}
"
, string.Join(Environment.NewLine, rules.Select((i, o) => string.Format(@"
if (!_done) {{ rule{0}(state); }}
", o)))
, string.Join(Environment.NewLine, rules.Select((i, o) => string.Format(@"
private static void rule{0}(InstrumentState state) {{
if ({1}) {{ {2} }}
}}", o, i.Trigger, string.Join(Environment.NewLine, i.Actions))))
);
var types = new[] { typeof(Console), typeof(InstrumentState), typeof(ErrorEventAction), typeof(ComponentId), typeof(global::StateMachine.Types.State) };
var refs = types.Select(h => MetadataReference.CreateFromFile(h.Assembly.Location) as MetadataReference).ToList();
//some default refeerences
refs.Add(MetadataReference.CreateFromFile(Path.Combine(Path.GetDirectoryName(typeof(System.Runtime.GCSettings).GetTypeInfo().Assembly.Location), "System.Runtime.dll")));
refs.Add(MetadataReference.CreateFromFile(typeof(Object).Assembly.Location));
var parse = CSharpSyntaxTree.ParseText(code);
var assyName = Guid.NewGuid().ToString();
var options = new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, allowUnsafe: true, optimizationLevel: OptimizationLevel.Release);
var compilation = CSharpCompilation.Create(assyName, new List<SyntaxTree> { parse }, refs, options);
var state = Expression.Parameter(typeof(InstrumentState), "state");
Action<InstrumentState> y = (_) => { };
using (var stream = new MemoryStream())
{
var result = compilation.Emit(stream);
if (!result.Success)
{
var compilationErrors = result.Diagnostics.Where(diagnostic =>
diagnostic.IsWarningAsError ||
diagnostic.Severity == DiagnosticSeverity.Error)
.ToList();
if (compilationErrors.Any())
{
var firstError = compilationErrors.First();
var errorNumber = firstError.Id;
var errorDescription = firstError.GetMessage();
var firstErrorMessage = $"{errorNumber}: {errorDescription};";
var exception = new Exception($"Compilation failed, first error is: {firstErrorMessage}");
compilationErrors.ForEach(e => { if (!exception.Data.Contains(e.Id)) exception.Data.Add(e.Id, e.GetMessage()); });
throw exception;
}
}
else
{
stream.Seek(0, SeekOrigin.Begin);
var assy = AssemblyLoadContext.Default.LoadFromStream(stream);
var type = assy.GetType("SomeCompany.SomeProject.Instrument.State.Reducers.Script");
y = Expression.Lambda<Action<InstrumentState>>(Expression.Call(type.GetMethod("Execute"), state), state).Compile();
}
}
return y;
}
You know when the first line creates a variable code
and it holds C# code, you're in for a time.
The first block of code here does a lot. It queries the rules
object- a reference to our settings database- to generate a series of rule{0}
functions, where the {0}
is some name. The body of the function has a condition on i.Trigger
(ensuring this rule only applies sometimes) and then has a body defined by i.Actions
, which is just C# code living in our data source.
We then populate an Execute
function with calls to every rule{0}
function we generated. These themselves are further gated by a _done
check, meaning it's possible for some rules to abort the processing of further rules.
The rest of the code here is a set of interactions with the C# compiler. We parse and compile the C# code that we just munged together through string concatenation, emit that compiled data into an in-memory stream, and if it succeeds in compilation, we create a C# assembly based off that stream. That is to say, we generate and execute a library without leaving anything on the filesystem for further review. It all exists purely in memory.
We then generate a lambda which calls the Execute
function in that newly generate library, and return it, so that other callers can now use it.
There are so many things wrong with this. Setting aside the code generation, the code that gets generated is complicated: a chain of statements where each has its own dedicated trigger condition and may be skipped based on a done flag. Just trying to analyze that for any non-trivial set of rules is hard.
The code generation, however, is the real WTF. First, they were using a set of static analysis tools to try and maximize code safety. None of the code living in the settings database went through that. Second, the settings database was editable by customers. Doctors were expected to edit it. The very idea that you could change the code running on the device by editing the Settings database was a huge "No!" But the bonus of doing this all in memory means that if there were a breach, it'd be trivially easy for an attacker to cover their tracks.
Now, there was no malice on the part of the tech lead. This was all just for personal convenience to speed up iterating on the code. It wasn't an intentional security flaw- it was just clever.
Sally raised this to her boss. He sighed, put his head in his hands, and was silent for a long moment. "We just signed off on doing pen-testing last week. They're going to destroy us."