`removeDeadNode` Function Issues with Conditional Statements
Closed this issue · 8 comments
Description:
When running the removeDeadNode
function on specific code snippets that contain conditional statements, two issues arise:
-
For the snippet:
var J; if (J==0) var x,z; else var y; console.log("hello");
An error is thrown indicating a failure in generating code because
consequent
branch of the if statement isundefined
. -
For the snippet:
var J; if (J==0) { var x,z; } else{ var y; } console.log("hello");
The output contains empty
if
andelse
blocks:var J; if (J == 0) { } else { } console.log('hello');
Possible Solutions:
-
Enhance
removeDeadNode
to better handle conditional statements without braces. -
Add a new module to remove empty
if
andelse
blocks when deobfuscating, if that is considered ideal behavior.
Again, I'm not certain which approach is best, but I would appreciate your expertise on how to best solve this issue. Thank you for your continuous efforts to improve this invaluable tool.
I'm pushing a fix for this in flAST. I'll update when it's out.
Good catch, thanks!
Btw, output containing empty if-else blocks are the intended behavior. Consider the following:
var J;
if (J = true) {}
else {}
console.log(J ? 'success' : 'failure');
The test clause can contain any type of code that might have side effects, while returning either a truthy or falsy values, so it's hard to determine if it can be safely removed. I this the deobfuscation should err on the side of caution.
I believed this is solved by #91
Thank you so much for your swift response, and for your relentless efforts in refining this important tool.
I completely understand the concerns you raised about the potential side effects that could occur in the test clause. My apologies if my previous explanation was not as clear as it could be. What I was attempting to suggest was that in certain, well-defined scenarios, the conditional blocks can be safely simplified without altering the program's behavior. For example:
if (J == 0) {} else {}
can potentially be reduced toJ == 0;
if (J == 0) {} else { //elsebody }
can be transformed toif (!(J == 0)) { //elsebody }
These changes ensure that no side-effects could occur, thus preserving the original intent of the code while also making it more succinct.
I haven't thought of just keeping the test clause - that's a good idea!
Also, negating the if test clause to keep the else is also a good idea! I'll adopt both!.
I'm reopening this issue.
Thanks again for your attention and your great suggestions!
I believe that simplifying these if statements should be its own module, and not part of the removeDeadNodes
module
Hopefully this was solved with the new simplifyIfStatements
module (#92)
Yes, I agree that this should be a separate new module. Thank you for your assistance. In dealing with obfuscated js files, I've also employed two approaches to simplify if
statements.
For example, transforming nested if-else
blocks:
if (condition1) {
// ...
} else {
if (condition2) {
// ...
} else {
if (condition3) {
// ...
}
}
}
into if-else if
constructs:
if (condition1) {
// ...
} else if (condition2) {
// ...
} else if (condition3) {
// ...
}
And, turning nested if
conditions:
if (condition1) {
if (condition2) {
if (condition3) {
// ...
}
}
}
into combined conditions:
if (condition1 && condition2 && condition3) {
// ...
}
Both techniques reduce awkward indentation and improve code readability in my result. I believe these can also be incorporated into the new module.