Critical bugs in the LLM identifier renaming path (context slicing & string-based visited set)

[123wwwa]

Description

Hello! Thank you for open-sourcing the artifact for your NDSS 2026 paper, “From Obfuscated to Obvious: A Comprehensive JavaScript Deobfuscation Tool for Security Analysis.”

While reviewing visitAllIdentifiers and its helper functions, I found two correctness issues that can omit relevant context and skip independent bindings. I also noticed a potential performance bottleneck caused by repeatedly generating code from the AST.

I initially reproduced these cases using a JavaScript transcription of the implementation. I have since confirmed all four correctness cases and the successful-renaming control case by directly importing the repository’s original TypeScript implementation, using its locked dependency versions. The tests used a deterministic mock visitor instead of an LLM. Neither the implementation nor the lockfile was modified.

Test environment

• Repository commit: eaa72ae63758957ac95b2f5e3391fefb96827987
• Platform: Windows x64
• Node.js: 24.21.0
• npm: 11.19.0
• @babel/core: 7.26.9
• @babel/traverse: 7.26.9
• @babel/types: 7.26.9
• tsx: 4.20.3

The installed dependency versions above match package-lock.json.

Bug 1: Context slicing can omit the target identifier in both local and program scopes

In scopeToString, long non-program scopes are truncated from the beginning, regardless of the target declaration’s position:

if (surroundingPath.isProgram()) {
  // Attempts to center the slice using original source offsets.
  // ...
} else {
  return code.slice(0, contextWindowSize);
}

Local-scope reproduction

const code =
  "function F(){" +
  "void 0;".repeat(400) +
  "let target=1;return target;}";

const calls = [];

await visitAllIdentifiers(
  code,
  async (name, context) => {
    calls.push({ name, context });
    return name;
  },
  1000
);

const targetCall = calls.find(call => call.name === "target");

console.log(targetCall.context.length);             // 1000
console.log(targetCall.context.includes("target")); // false

The visitor receives the target name separately, but its code context contains neither the target declaration nor its uses. This removes evidence needed to infer a meaningful name; it does not necessarily guarantee inference failure.

Even when a declaration appears near the beginning of a function, important uses later in the function can still be excluded.

Program-scope reproduction

The program-scope branch uses mismatched offsets:

const code = `${surroundingPath}`;
const start = path.node.start ?? 0;
const end = path.node.end ?? code.length;

path.node.start/end describe positions in the original source, while ${surroundingPath} generates a new string from the current AST. Formatting changes and earlier renames can move the target in that generated string without updating the original offsets.

I reproduced target omission in the program branch with:

const code =
  "0;".repeat(600) +
  "let target=1;consume(target);" +
  "1;".repeat(600);

With contextWindowSize = 100, the context passed for target contained only preceding statements and no target. It was also 106 characters long, because the central-slice calculation adds half the budget on each side of the identifier’s original span.

Suggested fix

Use target positions that correspond to the exact string being sliced, accounting for the selected scope and prior AST changes. Alternatively, build context around the target declaration and relevant references to the same binding.

Copying the current program-scope logic into the local-scope branch is insufficient:

• Original-source offsets do not reliably match generated-code offsets.
• Local-scope strings also require scope-relative positions.
• Window boundaries should respect the configured character budget, with explicit handling when the target itself exceeds it.

Bug 2: Name-based visit tracking skips independent bindings

The visited set tracks names across one visitAllIdentifiers invocation rather than lexical binding identities:

function hasVisited(path: NodePath<Identifier>, visited: Set<string>) {
  return visited.has(path.node.name);
}

// After the optional rename:
markVisited(smallestScope, smallestScopeNode.name, visited);

scope.rename() mutates the identifier node. After a successful rename, smallestScopeNode.name normally contains the new name, not the original name.

Therefore, renaming one a does not automatically skip all subsequent variables named a. The following cases do reproduce the problem.

Case A: Leaving one name unchanged skips an independent binding with that name

function A() {
  let a = 1;
  return a;
}

function B() {
  let a = 2;
  return a;
}

With a mock visitor that returns its input name unchanged:

1. The visitor processes A’s local a and returns "a".
2. "a" is added to visited.
3. B’s independent local a matches that entry.
4. Its visitor call is skipped.

Observed: Only one visitor call for a, despite there being two independent bindings.

This can also compound Bug 1: if insufficient context causes the visitor to retain a name, later independent bindings with that name are skipped.

Case B: A renamed binding suppresses an existing independent binding

If an earlier binding is renamed from a to itemCount, the set records "itemCount". A later, independent binding already named itemCount can then be skipped.

I reproduced this using a larger A function to ensure its local a is processed before B’s local itemCount:

const code =
  "function A(){let a=1;" +
  "void 0;".repeat(20) +
  "return a;}" +
  "function B(){let itemCount=2;return itemCount;}";

The mock visitor returns "itemCount" for "a" and otherwise returns the input name.

Observed: A’s a is processed, but the visitor receives zero calls for B’s existing itemCount.

Control case: Successful renaming does not automatically skip the next a

For the two-function example in Case A, returning "itemCount" for each "a" produces two visitor calls for a. The resulting local names are itemCount and _itemCount.

This confirms that the failure depends on the names actually recorded in visited.

Suggested fix

Track Babel binding identity, capturing the binding before renaming:

import type { Binding } from "@babel/traverse";

const visited = new Set<Binding>();

for (const path of scopes) {
  const binding = path.scope.getBinding(path.node.name);
  if (!binding || visited.has(binding)) continue;

  // Existing context extraction, visitor call, and rename logic.
  // ...

  visited.add(binding);
}

The canonical binding.identifier is another suitable identity. Tracking arbitrary declaration nodes is less robust because multiple declarations can belong to the same binding.

Progress reporting should also use the number of unique bindings if processing is changed to operate per binding.

Reproduction results

Test	Observed result
Local declaration after a long prefix	Target absent from the 1,000-character context
Program context sliced using original offsets	Target absent; 106 characters returned for a 100-character budget
First a left unchanged	Second independent a skipped
First a renamed to a later independent binding’s existing name	Later binding skipped
Control: first a renamed successfully; second binding also named a	Both bindings processed

All five tests passed assertions confirming the reported behavior, including the control case. Passing these reproduction tests means the observed behavior matches the report; it does not mean the bugs are fixed.

These results were confirmed against the repository’s original implementation and locked dependency versions at the commit listed above. The deterministic tests establish context-selection and binding-tracking failures without relying on LLM behavior. They do not measure downstream naming quality, failure rates, or performance overhead, and I have not established whether these issues affected the paper’s reported results.

Additional Performance Concern: Repeated full-scope code generation

In addition to the correctness issues above, scopeToString regenerates the entire selected scope for every processed identifier:

const code = `${surroundingPath}`;

The context limit is applied only after this string has been generated. Consequently, a small contextWindowSize does not bound the amount of AST printing performed.

When K identifiers select the same large scope of size N, this can result in approximately O(K × N) code-generation work, along with repeated string allocations and garbage-collection pressure. For inputs where both quantities grow proportionally, this part of the implementation can exhibit quadratic scaling.

This is a potential performance bottleneck for large inputs. The reproduction tests above establish correctness failures, but do not benchmark this overhead or its share of total runtime relative to LLM requests.

Suggested improvement

Avoid regenerating a complete scope solely to extract a short context window. Possible approaches include:

• Reusing the original source, which is already available as the code argument, with its matching original AST offsets.
• Maintaining a cached source representation with position mappings updated as edits are applied.
• Generating only the relevant declarations and references needed for the context.

The implementation should explicitly decide whether later prompts must reflect earlier renames. Original-source slicing preserves valid original offsets but does not include those renames; it therefore requires keeping the original target identity and name aligned with the prompt. A representation that reflects ongoing changes requires position updates or appropriate cache invalidation.

Simply caching each scope’s generated string is insufficient if later AST mutations can make that string stale. Likewise, reusing original offsets against updated generated code would retain the coordinate mismatch described in Bug 1.

Questions About the Paper's Failure Analysis and Performance Attribution

The implementation issues above also raise questions about how to interpret the paper's reported failures and timing results. These are questions about causal attribution, not claims that the reproduced bugs explain the reported error rates. Confirming behavior at the repository commit listed above does not establish that the same code and settings were used in the experiments.

1. Context construction as a possible contributor to the reported hallucination rate

Section V-I (pp. 11–12) reports a 12% hallucination rate in an evaluation using 100 samples and GPT-4o-mini, describing cases such as overly verbose identifier names as over-interpretation.

Bug 1 provides a plausible alternative or contributing explanation for some naming errors: the prompt may omit the target's declaration and relevant uses. In that situation, the observed result reflects both the model and the context-construction implementation. The current reproduction tests do not establish how often this occurred in the evaluation or whether it accounts for any particular reported failure.

Could you clarify whether the evaluated version used this context-selection logic and whether the failed requests retained the target declaration and relevant references? Comparing the original and corrected context construction on the same samples, with the model settings and evaluation criteria held constant, would help separate implementation effects from model limitations. Repeated runs would also help account for model variability.

2. Appendix H includes a structural change that identifier renaming alone does not explain

Section V-E (p. 10) reports 17 semantically inconsistent samples out of 100 and identifies LLM-based variable renaming as the primary cause. Appendix H (pp. 17–18) illustrates an undefined idx in Listing 7.

However, the relevant expressions in Listings 6 and 7 differ structurally, not just in identifier names:

// Listing 6: the computed property is a function call.
arr[_0xgetMethod(0)](...)

// Listing 7: the computed property is an array access.
numbersArray[arrayMethods[idx]](...)

The function call _0xgetMethod(0) has become arrayMethods[idx], introducing a reference to idx outside its original parameter scope. The supplied visitAllIdentifiers rename logic does not itself perform this call-to-member-expression transformation.

Also, Bug 2 skips processing an independent binding; skipping its rename normally leaves both its declaration and references unchanged. It does not, by itself, demonstrate the partial renaming or structural substitution needed to explain this example. I therefore do not attribute the Appendix H failure or the reported 17% inconsistency rate to Bug 2.

Could you identify which transformation produced arrayMethods[idx] and provide the relevant intermediate code immediately before and after the Humanizer? In particular, was the undefined reference already present in the Humanizer input? Since the appendix listings are abbreviated, a complete reproducer and stage-by-stage outputs would help determine whether the problem arose during an earlier transformation, identifier renaming, or another step.

3. Humanizer overhead should be separated from API waiting time

Section V-G and Table V (pp. 10–11) report an average of 9.23 seconds for the configuration with preprocessing and static/dynamic deobfuscation, versus 87.31 seconds for the full system. The text attributes the increase to LLM API latency.

The 78.08-second difference measures the additional wall-clock cost of enabling the full Humanizer configuration. The table alone does not separate API waiting time from local context generation, AST renaming, formatting, or other overhead. Repeated full-scope code generation is therefore a cost worth measuring, but its presence does not establish that it dominates runtime.

The separate 100-sample reliability experiment in Section V-I reports 20,897 API calls, 10.96 hours in total, and a mean of 6.6 minutes per sample with a median of 10.7 seconds. Those results concern a different evaluation set and should not be treated as interchangeable with the 87.31-second ablation average.

Could you provide a timing breakdown for context generation, AST updates/output generation, and API waiting? A deterministic mock-visitor benchmark, alongside measurements with actual API requests, would help quantify the local processing overhead and test whether caching or targeted context extraction materially improves runtime.

These checks would help distinguish reproducible implementation defects from model limitations and clarify their impact on the reported results without assuming a cause in advance.

Additional Note on Token Efficiency and API Calls

The implementation awaits a separate visitor call for each unvisited candidate. If each call makes an LLM request, nearby identifiers can repeatedly transmit overlapping context.

In my project, FlowName, I am exploring a relation-aware batching approach: using lightweight semantic relations, such as lexical bindings, assignments, and property accesses, to group related identifiers and request multiple names within a shared context budget.

This could reduce duplicated context and request count, although the effects on cost, latency, and naming quality require benchmarking. Binding identity and scope-aware collision handling remain necessary when applying batched results.

I am sharing this as a possible optimization direction, separate from the correctness issues above.

Thank you for considering these findings. I would be happy to discuss a patch or contribute regression tests.