MichaelXF / js-confuser

JS-Confuser is a JavaScript obfuscation tool to make your programs *impossible* to read.
https://js-confuser.com
MIT License
236 stars 35 forks source link

Obfuscation methods are using patchable functions that makes code unsafe #132

Open doctor8296 opened 4 months ago

doctor8296 commented 4 months ago

Hello! First of all I want to say that this tool is great. Previously I was using only javascript-obfsucator, but it was extremily easy to deobfuscated and debug. Some people even did whole deobfuscators based on the obfuscation logic of the javascript-obfuscator.

Okay, now I will give a little bit of context. I am web game anticheat dev. My client side anticheats were used in multiple games, and it was pretty effective, even tho it is just uncopmlite version of actual anticheat (actual versioun requires a lot of server related work which is extremily difficult and have a lot of issues at implementing it on ready made games + plus it has bad relationships with hashing).

So, since I was using javascript obfuscator on custom low obfuscation config, because I couldn't use any other since it took huge chunk of performance (like 2 seconds, which is unacceptable for the game or site), for hackers it was very easy to handle it "on fly" and edit specific places in code to make it work like they wanted. Even tho they did it I was very happy that they were forced to do this out of options, instead of trying to monkeypatch this (basically my anticheat protects from monkeypatching).

So I wanted to make it stronger. And in you library I see a lot of great features for that, but unfortunatly a lot of them use patchable function to make them work.

Patchable functions are methods / global functions / global classes that can be redefined. Here the example of one:

const S_charCode = "S".charCodeAt(0);

And this function (method String.prototype.charCodeAt) can be patched, for example like this:

String.prototype.charCodeAt = function() {
    return "fake value"
}

And this is unsafe.

The live example of it is source protection. I actually use hash source protection that wraps all the completed / obfuscated code into function, then I am getting the hash of this function and pass as argument. Inside of this function I am getting the function signature and compare it's hash with passed hash. Obviously to hash it I have to use chatCodeAt to get char code and base hash value on it. I did it by creating static dictionary of all existing symbols:

      return {
        0: 48,
        1: 49,
        2: 50,
        3: 51,
        4: 52,
        5: 53,
        6: 54,
        7: 55,
        8: 56,
        9: 57,
        ":": 58,
        ";": 59,
        "<": 60,

Alright. Now I'll show some specific case of the methods of the JS-Confuser which we can make safier.

Overall this thing is great. I cannot ask you to implement this things that I meantioned, probably gonna do it myself. Web security and monkeypatching is very difficult topics, that no one actually know about and only a few neet it.

Little comments:

Documation12 commented 3 months ago

lot to read lol

doctor8296 commented 3 months ago

lot to read lol

"Didn't read, lol"

doctor8296 commented 3 months ago

@MichaelXF by the way I found few bugs when code breaks even with no enabled options at all... seems like prettifier issue. I can provide full data about specific cases if you want.

Documation12 commented 3 months ago

I think this project is abandoned, nice dc server

doctor8296 commented 3 months ago

I think this project is abandoned, nice dc server

Thit is unfortunate.

MichaelXF commented 3 months ago

Patchable functions are methods / global functions / global classes that can be redefined. Here the example of one:

const S_charCode = "S".charCodeAt(0);

And this function (method String.prototype.charCodeAt) can be patched, for example like this:

String.prototype.charCodeAt = function() {
    return "fake value"
}

And this is unsafe.

This is absolutely something the deobfuscator should protect against. I plan to make JS-Confuser safe from this reverse engineering strategy

  • String compression ... If not replacing them, it could be amazing to have customisable function for encoding and other string-operate functions (such as String concealing, String encoding)!

Yes, this is planned.

  • Duplicated literal removal ... It has OVV74A9['call'](this) for no obvious reason. I can imagine only one case, for example when function returns array that has arrow function... I am not sure, but apperantly it has a reason, but it still stays unsafe.

The .call(this) will be removed as it's unsafe.

  • Dispatcher It has similar to Duplicated literal removal issue, but it also uses spread operator in arguments. And yeah... spread operator is also hackable. image

Wow classic JavaScript. We will have to deter from spread operator now.

  • Dead code Little comment on Dead code option image Deobfuscators (such as deobfuscate.relative.im) can easily find such expressions that are obviously "dead". You can make it more difficult for deobfuscators by creating check when even human can never be sure. For example "key123" in {} (we can never be sure that prototype hasn't this property defined... well... yeah it can break someones code, but who defines something on global prototype anyway, right? I can modify this example).

Yes Dead Code should place more complex false tests. The current idea is other transformations (Duplicate Literals Removal) will obfuscate the false to something more unreadable. I like "key123" in {}. Maybe this could integrate well with Opaque predicates

  • Stack / Flattern Another very interesting option. But it has unsafe spread operator use, defineProperty (for some reason).

The Object.defineProperty is purely used to preserve the original function.length property. A new option named preserveFunctionLength will allow users to disable this feature in the next release.

  • Rgf It has huge potential but it is weird and unsafe in both term.

RGF has some issues with variable scoping but the next release contains some improvements for it.

Little comments:

  • _=>_=>_=>...=>_ <- this thing will kill obfsucator.

It also errors in my console and Node.JS, is this a valid program? Additionally acorn and escodegen are responsible for all parsing / generating, so it's not an issue JS-confuser can easily solve.

  • debugger stuff is not that effective against monkeypatching at all (we can just manualy disable setInterval and fake Date.now)

What would be a better anti-debugger approach?

  • do you use hashing for integrity check?

Yes.

  • footer on js-confuser.com is hidding very buttom of the code

That is intended. After obfuscating the code you are prompted to download it or run it in the browser. The web version is mostly for demoing purposes, ideally you'd want to integrate JS-Confuser in your build tools.

  • if you select shuffle false on js-confuser.com it will still try to shuffle arrays with length > 2

Great find. The shuffle option was being set as "false" which evaluates as a truthy value. This will get fixed.

doctor8296 commented 3 months ago

@MichaelXF Hi! Thank you for you response! I will glad to help you make your obfuscator more safe and less patchable To be honest I can talk for hours about patching / hacking / scripting / protecting js related stuff :P So, be free to ask anything, I will try to answer 👍

Little response: _=>_=>_=>...=>_ this is actually a valid program, it is just an arrow function _=>_ that return other arrow function _=>_ and if you will do this long enough the deobfuscator/obfuscator will have stack overflow error.

MichaelXF commented 3 months ago

Sweet. Thank you for your expertise, this will help improve JS-Confuser :)

Feel free to create more issues if you find more unsafe / 'hackable' behaviors. I also have more time to spend so I can maintain JS-Confuser better.

Documation12 commented 3 months ago

Can you make integrity more secure?

MichaelXF commented 3 months ago

Can you make integrity more secure?

Yes I'm sure theres some improvements for it. Let's push that to a new issue

Documation12 commented 3 months ago

Ok will do

doctor8296 commented 3 months ago

@MichaelXF can I continue post here my findings / more safe methods / bugs without creating new topic everytime?

Here the "bug" I found:

Screenshot 2024-08-05 at 11 51 53

The Zero width rename type adds unecessary zero width character each time, which cause big file size.

I am suggesting to regenerate array each time it is out of keywords and do count++

like this:

let count = 1;
const keyWords = ["for", "if"];
let array = [];
const getNextVarName = () => {
    if (array.length > 0) {
        return array.pop() + "\uZeroWidth".repeat(count);
    } 
   ++count;
   array = gen(keyWords) // shuffles key words array to a new one
   return getNextVarName();
}

We also can use other reserved words in form of premitives:

true, false, undefined, null, NaN etc.

function anonymous(
) {
const true‌=10; return true‌
}
doctor8296 commented 3 months ago

@MichaelXF Also, I was wrong about "spread operator" in functions like this:

function f(...args) {
   // code
}

It doesn't actually gets handled, because it is not an actual spread operator

Screenshot 2024-08-05 at 13 25 44

But the actual one does:

Screenshot 2024-08-05 at 13 28 14
MichaelXF commented 3 months ago

@MichaelXF can I continue post here my findings / more safe methods / bugs without creating new topic everytime?

Yes

Screenshot 2024-08-05 at 11 51 53

Good find, this is an easy fix. But realistically just be a normal person and use randomized or mangled variable names 😂

We also can use other reserved words in from of premitives:

true, false, undefined, null, NaN etc.

function anonymous(
) {
const true‌=10; return true‌
}

This can be added too

doctor8296 commented 3 months ago

Good find, this is an easy fix. But realistically just be a normal person and use randomized or mangled variable names 😂

Well, manage such thing as zero length characters is a little bit harder. Personally I tried to auto-deobfuscate some code with it and failed. Because a lot of editors / copy functions doesn't work with zero characters length that good. And if this makes process even a little bit harder - I am gonna use it :P

doctor8296 commented 3 months ago

@MichaelXF Here interesting JS behaviour that you can use for fancy obfuscation methos:

(+{
    valueOf() {
        // will get triggered
    }
});
("" + {
    toString() {
        // will get triggered
    }
});

This has very interesting look in debugger: image

That fact that some functions gets triggered by just math operators is pretty interesting

doctor8296 commented 3 months ago

Here other ways to get global method that I mentioned:

In this case we are using constructor of the generator function image

Howerver, even though GeneratorFunction constructor is not global (it doesn't exists on window at all) it still can be redefined before. And not only constructor itself, but Symbol.iterator as well.

Here the way of how to hack it: image

doctor8296 commented 3 months ago

@MichaelXF Btw, here size-effective generator for zero-length variables! I have also added few keywords. I think code with such setting will be 3-times smaller now!

var keywords = [
  "if",
  "in",
  "for",
  "let",
  "new",
  "try",
  "var",
  "case",
  "else",
  "null",
  "break",
  "catch",
  "class",
  "const",
  "super",
  "throw",
  "while",
  "yield",
  "delete",
  "export",
  "import",
  "public",
  "return",
  "switch",
  "default",
  "finally",
  "private",
  "continue",
  "debugger",
  "function",
  "arguments",
  "protected",
  "instanceof",
  // "function", <--- function is repeating !
  "await",
  "async",

  // new key words and other fun stuff :P
  "NaN",
  "undefined",
  "true",
  "false",
  "typeof",
  "this",
  "static",
  "void",
  "of",
];

var maxSize = 0;
var currentKeyWordsArray = [];

function shuffle(array) {
  var currentIndex = array.length;
  while (currentIndex != 0) {
    var randomIndex = Math.floor(Math.random() * currentIndex);
    currentIndex--;
    [array[currentIndex], array[randomIndex]] = [
      array[randomIndex],
      array[currentIndex],
    ];
  }
}

function generateArray() {
  var result = keywords
    .map(
      (keyWord) =>
        keyWord + "\u200C".repeat(Math.max(maxSize - keyWord.length, 1))
    )
    .filter((craftedVariableName) => craftedVariableName.length == maxSize)

  if (!result.length) {
    ++maxSize;
    return generateArray();
  }

  return shuffle(result);
}

function getNextVariable() {
  if (!currentKeyWordsArray.length) {
    ++maxSize;
    currentKeyWordsArray = generateArray();
  }
  return currentKeyWordsArray.pop();
}
doctor8296 commented 3 months ago

@MichaelXF do you have a discord or any other means of information exchange for private protection-related topic?

MichaelXF commented 2 months ago

@doctor8296 Yes, Send me anything interesting here: michaelxfr@gmail.com

MichaelXF commented 2 months ago

Here is the first '2.0' rewrite for the frontend: https://new--confuser.netlify.app/

Please feel free to leave your feedbacks

doctor8296 commented 2 months ago

Here is the first '2.0' rewrite for the frontend: https://new--confuser.netlify.app/

Please feel free to leave your feedbacks

It doesn't work good with zero-length chars as previous one.

MichaelXF commented 2 months ago

It doesn't work good with zero-length chars as previous one.

That's somewhat a good thing as it breaks other Monaco based tools

doctor8296 commented 4 days ago

We can use source itself to decrypt strings

function decrypt(str, password) {
    // ...
}

const encryptedString = "ABC...";

decrypt(encryptedString, decrypt.toString());

the only thing which is required is that decrypting source shouldn't be inside of the "password" structure, basically because it is impossible

I love the idea of code unreversability rooted in such way so the code is structure dependend, so if you will break the structure - whole code will fall apart, so obfuscation will became not just messing code around, but a part of the code itself