mika-f
commented
1 year ago Thank you for your feedback.
Reply to some comments.
First of all, keep in mind that some code is not mine (that is, it is generally provided as a C++ library).
For example, assembly.hpp, byte_pattern.cpp, byte_pattern.h, injector.hpp are library code and have not been modified. Therefore, I will refrain from mentioning that part.
A lot of reinterpret_cast (which many consider bad practice to use)
I didn't know it was bad practice because I'm not a C++ professional. I would like to fix this if possible. You can also submit patches yourself, if possible.
Use of unions (even though the code is seemingly in C++17)
I will not mention this because it is an external library as mentioned above. However, I am personally working to stop relying on these libraries.
Heaps upon heaps of raw assembly
I also believe that raw assembly should be avoided from a maintainability standpoint, and I am gradually reducing it.
Someone could replace the DLL with a tainted one, and infect user's systems.
This seems like a word that can be taken in several ways. For example, regarding the use of DLLs in the context of Blender Add-ons, I've seen it in some other add-ons as well, so I don't think it's a problem (as for DLL Hijacking, I'm loading it from an absolute path, so Unintended DLLs should not be loaded, and assuming the DLL is replaceable, an attacker would be more efficient to replace the addon's entry point itself than to replace the DLL.) Next, the same thing can be said about loading code that the user does not know. Some well-known add-ons can be seen calling external processes internally or installing additional libraries. Since they contain implicit code, they are a security risk. Finally, I agree with you about using C++ code to replace the in-memory code. However, this is also a context that is not a Blender addon, for example, it is a common context in the mod culture in games (for example, Minecraft and Beat Saber add some modifications to the game itself, and the original localization addon is added to the game. in-memory patch).
Try to integrate this in Blender itself (i.e. add drag and drop to Blender itself without an addon).
This has been done by many developers and users over the years, and has been rejected by the Blender team, so this approach is hopeless. That's why this addon exists.
Clarify why such malware-like practice is needed, and give warnings to the risks of using this addon
I think this is a good idea. If you do this, I think it will be the following process, but if you have any opinions, I would be happy if you could add them.
- Install the add-on normally
- Added the following settings to the add-on settings screen
- A detailed description of what this addon is doing
- This addon uses C++ DLL code. Please check DLL publisher and DO NOT replace it.
- The C++ DLL hooks calls to certain functions in Blender.exe in order to receive events on drop. This is the desired behavior as Blender itself does not provide any events for drops.
- If you disable the add-on, these behaviors are restored.
- Read the explanation and agree
- A detailed description of what this addon is doing
- Where consent has been obtained, further processing in question
StandingPadAnimations
Kif11
Hey, so I noticed in the README it says:
To me, this is an extreme red flag, as replacing components with dynamic libraries is typically malware behavior. I went and looked at the C++ code itself and needless to say, I'm still concerned. Some things I found include:
reinterpret_cast(which many consider bad practice to use)Replacing components with dynamic libraries is in my opinion a terible idea security wise. Someone could replace the DLL with a tainted one, and infect user's systems. This is unacceptable, user security should be the number one priority (I say this as an addon developer myself)
I would advise considering whether or not the risks are worth it to users. Personally, I think either:
I look forward to your response addressing this concern