unified-naming-convention / NamingStandard

The standard for the Unified Naming Convention.
75 stars 54 forks source link

The environment check developer mixed up crypt.encrypt with crypt.custom.encrypt #22

Open RexiRexii opened 2 years ago

RexiRexii commented 2 years ago

crypt.encrypt that everyone uses accepts 2 args, a data and a key (both accepted as strings) crypt.encrypt( data, key)

however, the UNC env check test did it in a horrifying way local encrypted, iv = crypt.encrypt("test", key, nil, "CBC") which is unacceptable, it also has mixed up args. crypt.custom.encrypt( cipher, data, key, iv)

please do fix this, as it causes things to be really messy.

richie0866 commented 2 years ago

For some context, the test was written based on documentation provided in crypt.encrypt's error messages using Script-Ware:

image

RexiRexii commented 2 years ago

Here's the thing though, Synapse's formatting is different. None of the exploit developers can make it so it supports both formats. On another note: check out my PR, it fixes some stuff.

Exponential-Workload commented 1 year ago

Here's the thing though, Synapse's formatting is different.

Synapse does not support UNC (yet)

Unless they make a commitment to switch to UNC, you will need to do something like:

if syn and syn.crypt then -- In SynV3, a more robust & generally useful check would be something like: if identifyexecutor() == 'Synapse X' then
-- Use Synapse-specific Encryption | Use syn.crypt and NOT regular crypt, as some scripts and/or executors may polyfill syn.crypt to match Synapse-specific Encryption functions
else
-- Use Standard UNC Encryption
end

Alternatively, you can entirely drop Synapse support (what I would do in this case) using something like:

if syn and syn.crypt then -- checking for crypt as some executors may polyfill portions of the syn table, see last code block
return error('This script does not support Synapse!');
end;

Another alternative would be to implement AES in pure lua & fallback to your implementation when running on synapse & when crypt is not available


Imho, adapting the standard to Synapse's implementation of crypt is the wrong action to take; we should instead wait for Synapse's Developers to adapt UNC, as imo it's their responsibility to implement a UNC-compliant API, not the other way around

RexiRexii commented 1 year ago

ScriptWare V3 will surely adapt to the "usual" format (the one that I just previewed) UNC should be based on not ScriptWare, but what ALL the scripts in general rely on I am pretty sure if anyone used crypt.encrypt it would be in Synapse's format, since they use crypto++ for their crypt library.

Hence why you guys should chance the crypt library to the "usual" format.

pepsipriest commented 1 year ago

I was actually surprised by this in the process of forking my script for UNC-compliant exploits. Considering the "traditional" syntax is (was?) supported in a lot of environments, I thought it would also be supported in Script-Ware but apparently not. My original script targeted Krnl, which uses (or used?) the "traditional" syntax. This is not a big problem for me but I found it strange. However I could see it becoming a problem for exploits with less stringent argument checks since it could result in some values being mixed around.

Imho, adapting the standard to Synapse's implementation of crypt is the wrong action to take; we should instead wait for Synapse's Developers to adapt UNC, as imo it's their responsibility to implement a UNC-compliant API, not the other way around

I agree it shouldn't be adapted because UNC as a whole is forward-looking but it should at least include notes on how to implement support for exploits with legacy environment standards. A documented approach to backwards compatibility with non-UNC exploits as an optional annex to the UNC itself would be beneficial because a lot of new developers usually look at prior art to develop scripts, and there are really good chances that these scripts were written using Synapse X syntax. If these users base themselves on older code to build new stuff but see their scripts act wonky because of pure-UNC exploits having no backwards compatibility then they could resort to using Synapse X altogether, reproducing the problem. If you doubt this can happen then just take me as an example, I forked out 20 dollars to buy S^X despite the more modern Script-Ware because it allowed me to use familiar syntax and that's with me considering their subscription switch bullshit. If I did it then other people will probably do it

There's a lot of good tooling dedicated to UNC out there (especially rbxm-suite, kudos to its developer) but comparatively very little scripts compared to the 6+ years worth of content generated by S^X/Krnl users. Perhaps writing a page documenting the changes between a UNC and classical non-UNC exploits (I would call them "legacy exploits" since they are outdated) would be good to prevent these mistakes in the future and help new developers transition to the UNC ecosystem.