[sachin9058]

When a crypto object is used through an alias variable (e.g., var alias = aes; alias.Mode = CipherMode.CBC;), the engine lost the property settings because it compared variable names directly — alias never matched the tracked symbol aes.
Now the engine tracks alias assignments during tree conversion and resolves them when checking variable invocations.
Changes:

• Add alias map to CSharpBlockTree
• Track var X = Y; assignments in CSharpTreeConverter
• Resolve aliases in CSharpDetectionEngine before matching variable names
  Tests: Added DotNetAESAliasTest + ProbeCSharpAliasTestFile.cs. All 38 C# tests pass.

FIXES: #511

[Copilot]

🟡 Changes recommended

Moderate issues remain in nested-scope propagation, chained alias resolution, and initializer validation.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes C# crypto property detection when objects are accessed through alias variables.

Changes:

• Stores and tracks local C# aliases.
• Resolves aliases during variable matching.
• Adds AES alias regression coverage and a test fixture.

File summaries

File	Summary	Findings
engine/src/main/java/com/ibm/engine/language/csharp/tree/CSharpBlockTree.java	Stores block-level aliases.	None.
engine/src/main/java/com/ibm/engine/language/csharp/CSharpTreeConverter.java	Collects local alias assignments.	Moderate (2 votes): alias recording must require the complete initializer to be a single identifier; this applies at lines 141 and 161.
engine/src/main/java/com/ibm/engine/language/csharp/CSharpDetectionEngine.java	Resolves aliases during detection matching.	Moderate (1 vote): propagate enclosing alias scopes into nested blocks. Moderate (3 votes): resolve multi-hop alias chains transitively with cycle protection.
csharp/src/test/java/com/ibm/plugin/rules/detection/dotnet/DotNetAESAliasTest.java	Tests AES detection through aliases.	None.
csharp/src/test/files/rules/detection/dotnet/ProbeCSharpAliasTestFile.cs	Provides the alias test fixture.	None.

Review details

Suppressed comments (2)

engine/src/main/java/com/ibm/engine/language/csharp/CSharpDetectionEngine.java:101

• Because the converter creates a separate CSharpBlockTree for every nested block and the sensor analyzes each block independently, this map contains only aliases declared in the current block. For example, var aes = Aes.Create(); var alias = aes; if (ok) { alias.Mode = CipherMode.CBC; } is analyzed with an empty alias map for the setter, so the property is still lost. Propagate the enclosing alias scope to nested blocks (and add a regression test for this case).

            Map<String, String> aliases = blockTree.getAliases();
            for (CSharpTree statement : blockTree.getStatements()) {
                processStatement(traceSymbol, statement, aliases);

engine/src/main/java/com/ibm/engine/language/csharp/CSharpTreeConverter.java:168

• The alias map is populated from the declaration but is never updated when the local is reassigned. For example, after var alias = first; alias = second; alias.Mode = ...;, this map still resolves alias to first, so the setting is attached to the wrong detection (or omitted from second). Track simple identifier reassignments or invalidate the alias before resolving subsequent uses.

                            String aliasName = ctx.identifier().getText();
                            String originalName =
                                    ((CSharpParser.SimpleNameExpressionContext)
                                                    primaryExpr.primary_expression_start())
                                            .identifier()
                                            .getText();
                            if (!aliasName.equals(originalName)) {
                                aliases.put(aliasName, originalName);

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sachin9058]

@san-zrl @n1ckl0sk0rtge Hey can u please take a look on this PR ?

[san-zrl]

Passing this on to @fynnth. Could you review? Thanks.

[fynnth]

Hi @sachin9058

Nice fix for the direct-alias case, Thank you!
One gap I'd flag before merging, related to the Copilot comment on the earlier revision about "initializer validation": the alias map is only ever written, never invalidated on reassignment. visitAssignment bails out early (children.size() < 2) for a plain alias= ...; statement, so an earlier var alias = first; mapping survives forever, even after alias starts pointing at something else.

Repro:
var first = Aes.Create();
var second = TripleDES.Create();
var alias = first;
alias = second;
alias.Mode = CipherMode.CBC;

Mode=CBC gets attached to first (AES) instead of second (TripleDES), which is a misattribution, not just a miss. That's a regression in kind relative to pre-PR behaviour: before this change the setter was simply dropped for both objects (data loss); now it's silently attached to the wrong one (wrong data)
Suggested fix: handle the plain-reassignment branch in visitAssignment instead of just return null.

It would be great if you could handle that as well, otherwise i will include it my next Major pr and we can merge this for now as it is :)

[sachin9058]

Hi @fynnth

Got it thanks for flagging this. The reassignment case is now handled:

• visitAssignment updates the alias map on alias = second; and adds a CSharpIdentifierTree to the statements list so the engine can pick it up
• CSharpDetectionEngine.run maintains a mutable currentSymbol that gets updated when CSharpIdentifierTree statements are processed, so isInvocationOnVariable always compares against the correct variable
• resolveAlias follows alias chains transitively with cycle protection (var a = b; var b = c; → a → b → c)
• The initializer check only records aliases for plain variable references (not method calls or type names)
• The test file now uses a chain of aliases (first → second → alias) to verify the fix without triggering a second detection
  All 39 tests pass. Ready for your review whenever you're free.

[fynnth]

HI @sachin9058 ,
I tested the new reassignment handling and it seems to run into a similar problem now:

var first = Aes.Create();
var second = TripleDES.Create();
var alias = first;
alias = second;
alias.Mode = CipherMode.CBC;
Expected: only TripleDES gets Mode=CBC. Actual: both AES and TripleDES get it.

-->
09:31:54.126 [main] DEBUG com.ibm.mapper.utils.Utils -- [ enriched ] (BlockCipher) AES-CBC
09:31:54.126 [main] DEBUG com.ibm.mapper.utils.Utils -- [ enriched ] └─ (BlockSize) 128
09:31:54.126 [main] DEBUG com.ibm.mapper.utils.Utils -- [ enriched ] └─ (Oid) 2.16.840.1.101.3.4.1
09:31:54.126 [main] DEBUG com.ibm.mapper.utils.Utils -- [ enriched ] └─ (Mode) CBC

Could you have another look at your fix?
Best regards :)
Fynn

[sachin9058]

Hi @fynnth thanks for catching this !!! The issue was that aliases were being tracked per block and applied afterward. Because of this, when an alias was reassigned (like alias = second;), the old mapping wasn't removed, so the setter ended up being applied to both objects.

I've updated the logic to resolve the receiver as each statement is processed and remove the old alias mapping whenever a variable is reassigned. I also removed the alias state from CSharpDetectionEngine and CSharpBlockTree.

I've updated the reassignment test to use your exact repro. It now verifies that only TripleDES gets Mode = CBC and AES remains unchanged. All 39 C# tests are passing.

One limitation is that reassignments inside if/else branches are still handled in source order rather than based on control flow. We can look into that separately if needed.