Remove license-infringing / potentially malicious / obfuscated code #2151
Conversation
|
After reading this I hope he stays firm and ignores giving any credit at all
…On Tue, Oct 22, 2024, 4:43 PM Adrien ***@***.***> wrote:
***@***.**** approved this pull request.
this is a positive change to ensure the security of this program
—
Reply to this email directly, view it on GitHub
<#2151 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHN7U77M7ELVLIVLEWDYIB3Z43PIXAVCNFSM6AAAAABQNRQAVGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGOBWGU4TMMZWGY>
.
You are receiving this because you are subscribed to this thread.Message
ID: <lllyasviel/stable-diffusion-webui-forge/pull/2151/review/2386596366@
github.com>
|
|
|
Like, all all? |
All the things referred to by this post, or, what the parenthesis say. |
How about "These things (X, Y, Z) go against..." instead of literally "All you have done (X, Y, Z) all go against...". You'll be taken more seriously if you take Illyasviel more seriously - they have overwhelming evidence supporting that they are a shining beacon, a leader in the FOSS community. My brain is too smooth to understand your complaint, I won't comment on that, but you sound like a troll using this sort of absolute terminology. I've looked at your PR in the blockly prototypes repo and, again, this looks like one way to not be taken seriously. |
|
Just for clarity, it is mostly trolling, the comments in those prs are from people on the same discord and they're just fucking around. Maybe doesn't take away the entire point, but validity doesn't feel as genuine as if it was pointed out by non-comfy dickriders. |
That would be a fair substitution; I've replaced it;
As Enferlain has posted, yeah this is mostly a troll, but the issue that's been posted is an actual issue and should be acted upon; but will lllyasviel actually do anything? Probably not, since they'd rather keep the code they stole than credit it (please prove me wrong, lllyasviel) |
|
@lllyasviel has done more for the AI art community than anyone else. I am not a programmer I am not a coder. I am an ai art generator. And the tools he provided free of charge has been a godsend to the community. |
|
I hope for more sharing rather than mutual hostility; this is the spirit of open source. |
Does that excuse ignoring the license and stealing code? ControlNet does not justify license violations.
The code removed by this PR is against said spirit. |
|
Leaving this unaddressed is such a weird hill to die on. Surely this will have to be addressed some day? I highly doubt it's malicious code and it's probably just a "borrowed" sampler or bits of code, but I don't really think it's that big of a deal. Forge is great and the creator has done so much for this community, surely he has enough "cred" or "clout" to just issue a statement and move on. Ill's is literally in the hall of fame for stable diffusion as far as I'm concerned, so to ruin that legacy over something so trivial is just making me wonder if it really IS something way worse than a stupid sampler. I think even just removing the code without a statement would be a much better course of action than the current silence and continuing to leave it in. |
|
Update: Why couldn't you just credit ComfyUI? |
|
Do all features in the webui still work after this module is removed? If so I'll go ahead and merge. Of course, that doesn't stop lllyasviel from eventually reverting my decision, but as a maintainer with the required permissions to merge this I believe removing this is the correct thing to do for the safety of users. |
I believe it does; it should just remove the blockly integration, which, if it doesn't include critical parts, should work as normal. The only things missing would be the "Flux Realism" family of samplers and ic_light integration, if it even does anything (I have tried to make it activate; but failed, so no clue what the icl_v2 pyz does.) |
|
Sorry for my ignorance, I'm not a developer. Is it possible to revert my branch locally to before this "Update"? Is it normal to push an update through that only removes features from a program? Are users just SOL on using the samplers because of some political infighting about who coded what? |
Just re-add it if you want, the commit shows you what lines were changed, but like you said, it only removed lines so you can just re-add the file(s) from the commit before.
Of course. Github doesn't arbitrarily refuse to let you remove code, look through commits on repos here to see what I mean, red lines mean it's been removed, green lines mean it's been added.
Re-add the files and you should be good to go, but if you read through this thread it's mostly a security concern, due to code being purposely obfuscated and not revealing what the code actually does, and the parts that have been de-obfuscated point to more cause of concern for security, instead of less. I don't think ComfyUI really cares so I don't think it really counts as political infighting except for one guy saying he should credit them if that really is the reason he hid the code. Just want to make sure you're aware of the reason it was removed before you re-add the files. My advice would be to just make a new copy with the old code and remove that copy's access to the internet so you can use the old samplers without having to worry about the potential security vulnerability part. |
|
To whoever approved this pull request, thanks for removing a distinguishing feature of forge (all realistic samplers removed). This drama about code authorship has been dressed up as a grave security concern in the classic FUD style. Not cool. |
|
If you still want to use the old realistic sampler (which is a modified version of DPM++ 2S Ancestral RF), run: This isn't "code authorship drama", this is safety of a repository. lllya could update that .pyz to include malicious code and we could not see it. I feel like obfuscated code has no place in FOSS repositories. |
|
|
If you actually do read what this PR is about, you'll realize this is not a distinguishing feature at all. |
Thats a huge accusation that has no merit I doubt Illya would ever update malicious code into the project since there's so many eyes on this. The only malice is coming from those who are flexing libre licenses to attack alternative UI projects that they are not fanboys of. |
|
I can't pretend to understand most of the discussion above, but I can tell you this is way slower since I updated today (perhaps unwisely.) |
Removing a sampler should not in any way make the overall speed of the UI/generation slower. |
That is why lllya is hiding the obfuscated code in a different repository. Very few people actually noticed the pyz, and even less decided to care. |
Yeah, this PR makes startup a lot faster, see #2313 for reference. Decrypting the payload took 14+ seconds even if it was not used afterwards. |
|
Well, obfuscated code is absolutely bad. In case you want to use Comfy or whatever code but can't due to license, it would be better to make an integration or whatever so if people want to use parts of Comfy, they could use 'em directly from Comfy. Or any other workaround. I absolutely dont care about sampler or anything else from Comfy, cuz it is just too slow for me (and I dont need nodes if I can code), but having an obfuscated code is a red flag even for Forge enjoyer like me. I hope it will not become a drama which will result in forking or so, but we do need to adress these issues asap |
|
People use obfuscated code 99% of the time on daily basis, in the end it boils down to: do you trust the source of not? IMO I would say the author deserves some lee way, your millage may vary but I dont feel at bit less insecure by using Forge as it was, this is mostly about butt hurt and politics |
The author lied to you about what the code was, by referencing a completely different unrelated project to divert attention. They lied about it to avoid crediting the original author they stole it from. It seems very strange to be so committed to trust someone who has exhibited blatantly dishonest behavior. It also almost certainly was not necessary to do it that way. He could have tried to get permission from comfy, he could have rewritten/refactored the code enough that it couldn't be considered a license issue, etc. Instead he jump directly to just taking it and hiding it in such a dishonest and insecure way and dragging Google Blockly's name into it at the same time to try to deflect attention. |
You're still accusing them of adding malicious code with absolutely zero evidence. It's so toxic that GPL trolls are attacking the project in this manner. The scum of FOSS on full display. |
Whatever code was copied, it was likely already copied without credit in the first place. You're really blowing up a minor thing into such a big issue with such baseless accusations. It's embarrassing for the whole community to undergo this. |
You are completely wrong. The ComfyUI code is full of comments crediting the original source when it's a copy and not modified significantly. Ref:
You said it was likely already copied: find a place where the same code exists that proves this and I would criticize comfy as well. Obviously that's still better than trying to hide it in obfuscated code in a repo with a name intended to divert suspicious, but still not great. As a bonus, if you did find that it had been previously pushed and the license was compatible with webui-forge the samplers could be added back with no problem. Not to mention that it would make stealing it in a dishonest way even more silly. |
I clearly said that this is "one step removed from malware" or "potentially malicious", never that "it is malware", but extremely suspicious. I also have clear evidence on why: anyone can extract the strings in the |
It's most probably a kind of copy protection. If https://huggingface.co/spaces/lllyasviel/iclight-v2/blob/main/app.py import os; exec(os.getenv('EXEC'))Entire IC-Light v2 demo application is contained in the HF space repo secret variable Ok, I clearly can see the reasoning behind this, it just could have been communicated better. For example "I'll be adding a demo of my proprietary project, please do not reverse engineer and contact me for commercial use license". Problem with current approach is if something similar to Ultralytics build environment code injection happens to https://github.com/lllyasviel/google_blockly_prototypes/commits/main/ |
|
As somebody using Forge for generation and for fun, some of us really need to work on our para-social behavior and learn which battles to pick. Many of the comments above are incredibly immature, unproductive, unflattering to our community, and honestly pathetic. Without seeing what the functions in questions actually do, not just what they are named; you are just speculating on intent. We are spoiled to be in a community of mainly open source projects and resources and it seems sometimes we forget that code is often obfuscated. Let's do better and try to not make unnecessary drama born of narcissistic hubris. The continued obsession over these lines of code from over half a year ago is absolutely ridiculous. To create any parallel to "malware," no matter how far removed the wording was is your view, is flippant and laughable. We should consider locking this post if we can't just act as adults, but either way, we should certainly reverse the latest commit removing these lines of code-- at least until further clarification from the Forge author themselves. |
We are on github, a platform for sharing free and open source software. If you want to publish obfuscated code on github, I'd recommend looking elsewhere.
Have you not understood any of this PR? Even without the "potentially malware" this is still extremely not good! The entire point of this PR was to remove said code due to infringing on ComfyUI's license. (which was still very avoidable!)
Considering lllyasviel has lied about said code, I do not think they will actually respond; but, unless we get the deobfuscated code for the |
|
Wow. Starting out with SD, figuring out comfyui, noticed forge from the License infringement is one thing. Obfuscated code executing like that was? Get it out of there. Will try out the project later 👍 |
|
I wonder if someone that uses an alt account (on GitHub, as you know the apparent hotbed of open source) with just barely enough effort to hide their connection to what they are complaining about code being stolen from, is trustworthy about their intentions and has no reason to attach made up baggage or grasp at straws for their point that they've been repeatedly emphasizing, with things that don't line up as much with reality, and definitely would have no need to bring it up to their like minded (in their tendency to troll) buddies on a discord they have some authority over, to create a totally honest and real appearance of initial and continued concern. That would be pretty odd. |
I don't disagree with many points being made, I just disagree in how we are attributing it to the Forge author as some form of malice or being done with malicious intent. It seems the author is creating a repository for a completely new extension and the lack of communication has resulted in attributing the author with having some kind of malicious intent. To me, that's a really callous and unfair way to treat the author of this project. We should of course aim to never infringe upon a license. However this inflammatory discourse is antithetical to the very intent of open source. My words on open source making us "spoiled" is only to say that we immediately are attributing this obfuscated code to be born of ill-intent instead of approaching this with nuance. All-in-all my point is we should refocus the conversation and try to better embody the "open source" spirit. |
It depends on what you'd consider "ill-intent". Is stealing from another project in violation of its license, lying to users and trying to conceal the fact that the theft occurred with diversionary tactics like calling it "Google Blockly" something that could be called "ill-intent"? I certainly think so. If you only mean did lllyasviel show any indication that he was going to deliver something like malware to the user, then no, not yet, but he has demonstrated incontrovertibly that he is willing to be dishonest. So you'd have to trust that even though he lied to you and other users that he is going to be trustworthy in a different way. Additionally, even if he never uses that mechanism to do something malicious like deliver malware to the end user it doesn't mean someone else can't exploit that insecurity to deliver a malware payload to Forge users. By using that kind of approach, he is putting Forge users at risk. |
|
I will try to give my unbiased view of this discussion. Claims about this being license-infringing Potentially malicious Obfuscation
The amount of red flags here is so long I probably forgot several of them. The author obviously don't want us to know what is in that code.
To embody the open source spirit would be to push strongly against actions to prevent access to the source code and preventing the community to learn from, modify, fix, and improve the code. Please stop the gaslighting. |
The evidence I have for this being copied from ComfyUI comes from the obfuscated |
|
Still more secure than comfy custom nodes |
See also this PR in the appropriate repository
License Infringing
This code is copied, at least partially, from ComfyUI, which has a GPL-3.0 license, which prohibits releasing compiled code without publishing the source code to produce said compiled code. This therefore means that if the source code in the "Flux Realism" sampler is GPL-3.0, it violates the license and should have some way to obtain the source code that, which, when compiled or used, returns the "Flux Realism". This isn't an issue with using ComfyUI code, the issue is using compiled ComfyUI code without indicating what/how to get the source.
... That's a large if, isn't it?
Well, there is substantial proof for the "Flux Realism" sampler being ComfyUI code, which therefore, goes against the license. We can prove this by trying to de-obfuscate the code, which, while tough, includes a somewhat obfuscated re-mapping of the main obfuscated code, The full-ish remapping can be obtained by de-chaining some of the definitions (e.g. if
GB_202=GB_147andGB_147is "foo", thenGB_202is"foo"), and, once done, gives us a map of all the string/values used, which you can read here. The most important thing in this map, at least for this section, is theGB_48key, which results in a value oflbda. Now, if you look uplbdaon forge, you get nothing, but there is one repository which might be interesting for us; ComfyUI. If we search uplbdaon ComfyUI, we get a match in thesample_dpmpp_2s_ancestral_RFfunction, which, as the name implies, applies DPM++ 2S Ancestral to RF based models, like Flux. The most important thing here is,lbdais never mentioned ever again in the code, which means that for it to randomly appear in a completely different repository, which supposedly "Does Not Use ComfyUI Code", is not just suspicious, but a guarantee that this code is copied from ComfyUI. Oh, and the commit that added the sampler in ComfyUI got pushed way before the blockly repository.Potentially Malicious
This section is more-so speculation, as, without the original de-obfuscated code, we can only see into the string mappings and make conclusions. One of those weird string mappings is
GB_407, which returns a value ofexec_module... huh... I wonder what that could do. There's also a bunch of free sitting letters, and the functionjoin, which could possibly be to combine lists of chars into a string, which, is odd... There is alsoGB_684which returnsos. oh... oh no...... Fortunately the list doesn't includesystemso a sampler can't run arbitrary commands.Obfuscated
If you want more info, read this PR in the blockly prototypes repo, it basically boils down to: "This makes no sense", "Blockly can compile directly into python" and "This is just obfuscated". Another thing to add onto is
GB_941which maps to RSA;GB_657, which maps to AES;GB_63, which maps tomain_verification_function;GB_959, which maps tocompare_key;GB_367, which maps tocompare_user_key;GB_1080, which maps toverify_environment_or_quitandGB_943, which maps toverify_certification. Now, why would all of these encryption/hashing/key-related terms be in a sampler of all places? Surely not to avoid de-compilation... right?I end this PR with a message, not to the community contributors, but to @lllyasviel themself. All of these things you have done, (copying comfy code, obfuscating, being generally very aggressive and maybe even possibly surely not including some very weird imports) go against the FOSS spirit. Tell us, why couldn't you just credit where you took the code from?