parsify-dev / desktop

Next generation notepad-based calculator, built with extendibility and privacy in mind.
https://parsify.app
214 stars 2 forks source link

License is still easily circumvented #58

Closed ptrcnull closed 1 year ago

ptrcnull commented 2 years ago

Describe the issue User can use DevTools and override the return value of function requesting https://api.parsify.app/api/status/ to true.

Device (please complete the following information):

Additional context Suggested solution: rewrite license check in C/C++ and load it as a native module.

xxczaki commented 2 years ago

@ptrcnull Thanks again for reporting this! I just release version 1.9.1 with the suggested suggestion of yours implemented. Mind checking whether that fixes the issue?

ptrcnull commented 2 years ago

This... doesn't really change anything, provided that the module is named parsify-license and the function it exports is literally named isValid. It's still trivial to locate and patch out.

ptrcnull commented 2 years ago

Turns out current licensing mechanism needs only 6 lines of Python to patch out:

with open("app.asar", "rb+") as file:
    data = file.read()
    start = data.index(b"const i=async")
    length = data[start:].index(b";") + 1
    file.seek(start)
    file.write(b"const i=_=>true" + b" " * (length - 16))

I would consider this a critical issue that needs to be patched as soon as possible.

xxczaki commented 2 years ago

@ptrcnull Right, in 1.9.2 I reverted the failed attempt to patch this. I tried another approach this time - mind taking a look? I uploaded .deb and .AppImage here:

https://wormhole.app/5X0JB#V-nNLsjQe3AHX_2M_AzxMw

Note that there is a high change nothing will function correctly 😄

ptrcnull commented 2 years ago

It looks like whatever you're using to load .node files actually uses process.dlopen and by analysing the binaries there's one giant string that's present in all of them - sPrj8fKKYUeHPHtinzxG9WUDLJ3ooV7QNjTEhUdspzdpLdbmJw.

By replacing the function that uses dlopen with module.exports.sPrj8fKKYUeHPHtinzxG9WUDLJ3ooV7QNjTEhUdspzdpLdbmJw = _ => true, the checks can still be omitted.

EDIT: also, it looks like the old function that makes the request from JS is still in the code, but unused; not a security vulnerability though

xxczaki commented 2 years ago

I changed the way .node files are imported (don't know if it's better or not though):

https://wormhole.app/4Xoe1#-D7gQ3zBGewJPQIFPwnlpQ

EDIT: also, it looks like the old function that makes the request from JS is still in the code, but unused; not a security vulnerability though

Yeah, forgot to remove it.

ptrcnull commented 2 years ago

Just a nitpick, but you might consider building Linux modules as static, so they work on systems that don't use GNU libc.

ptrcnull commented 2 years ago

As for the latest version - it looks better, only thing I could suggest now is certificate pinning.

xxczaki commented 2 years ago

I've made some changes regarding the way the license is validated. However, I still need to think more about how to strengthen the whole thing to be harder to circumvent. Thus, I will leave this issue open for now.

ptrcnull commented 2 years ago

The only change in the previous command to patch the license are the auto-generated variable names - the license check mechanism still hasn't changed in 2.0.0-rc3

You can try this one: sed -i 's|var t=i(e);return n(t)|return {default:_=>11}|' /opt/Parsify\ Desktop/resources/app.asar

ptrcnull commented 1 year ago

sed -i 's/)).valid/))||true/'

...*rolls eyes*