PerimeterX/restringer

Generated code does not retain comments despite 'comment' option being set

lanvent opened this issue · 2 comments

I have noticed an issue where the generated code after deobfuscation does not retain comments, despite setting the comment option to true in both parsing and code generation phases. While this might not be crucial for deobfuscation per se, it does result in the loss of potentially useful comments when the tool is applied to batches of files that might include non-obfuscated code. The absence of these comments in the generated code can be detrimental for comprehensibility and debugging.

Issue Reproduction:

Here's how I currently see comments being handled:

const espree = require('espree');
const escodegen = require('escodegen');

const code = `
// This is a comment
function hello() {
  console.log("Hello, world!");
  // Another comment
}
`;

const ast = espree.parse(code, {
    comment: true,
    range: true,
    sourceType: 'script'
});

const generatedCode = escodegen.generate(ast, {
    comment: true
});

console.log(generatedCode);  // This does not retain the comments.

Proposed Solution:

Through some tests, I have found that adding the following lines seems to solve the problem:

escodegen.attachComments(ast, ast.comments, ast.tokens);

And also setting tokens: true in the parse options for Espree.

Here's how it looks:

// Your existing code
const ast = espree.parse(code, {
    comment: true,
    range: true,
    tokens: true,  // Add this line
    sourceType: 'script'
});
escodegen.attachComments(ast, ast.comments, ast.tokens);  // Add this line

// Your existing code

This seems to work perfectly for retaining comments. However, I'm not entirely sure if this has any other side-effects on the overall deobfuscation process.

Would it be possible to include this fix or provide feedback on whether it could potentially have any side-effects?

Thank you so much for your time!

I'd like to add a discovery I've made regarding the use of attachComments in the AST transformation process.

When replacing nodes within the AST, it is essential for the replacement node to inherit the range information from the node it is replacing. Failing to do so will result in the error: "Unable to apply changes to AST: attachComments needs range information."

For example, in a function like removeRedundantBlockStatements, we could modify the code to include this inheritance as follows:

const currentIdx = parent.body.indexOf(n);
const replacementNode = {
    ...parent,
    type: parent.type,
    body: [
        ...parent.body.slice(0, currentIdx),
        ...n.body,
        ...parent.body.slice(currentIdx + 1)
    ],
};
arb.markNode(parent, replacementNode);

This additional step ensures that comments can be correctly attached without errors. This inheritance behavior might be a useful addition to the flast library, potentially making the behavior more consistent across different transformations.

I believed this is solved by #91