Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Icon Loader dll glitch in windows device #30

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

GrimmDevcc
Copy link

Added a workaround for iconloader dll in windows 10 and up.

@PROMPTYLOL
Copy link

Looks nice. Good job.

@PROMPTYLOL
Copy link

@Sitois can we have update

@Sitois
Copy link
Owner

Sitois commented Dec 13, 2024

It's 6 am for me bro wait

@PROMPTYLOL
Copy link

bro please do this update. i need it badly

@Sitois
Copy link
Owner

Sitois commented Dec 13, 2024

What is your PR about ?

@GrimmDevcc
Copy link
Author

Basically in newer windows python handles image differently so I fixed it by directly calling the image using a shell. This may cause false positives tho

Copy link

@Lenochxd Lenochxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have identified several serious security and functionality concerns. As these changes introduce risks that could harm the project and its users, this pull request cannot be approved in its current state. Below is detailed feedback for each area of concern:


  1. The binary file nuclear_icon.png has been modified. Its purpose and integrity are unclear. Running a .png file using os.system is dangerous. If this file is not a legitimate image, it will execute harmful actions.

  2. Dangerous Use of os.system
    The following code raises major red flags:

os.system("nuclear_icon.png")
  • Executing a file blindly in this way poses a significant security risk.
  1. Removal of Safeguards in main.py
    The original code included logic to safely handle files:
with open('nuclear_icon.png', 'rb') as image:
    nuclear_icon = image.read()

This has been removed and replaced with unsafe execution using os.system.


Final Decision

This pull request introduces multiple serious security vulnerabilities and cannot be merged as is. Please address the feedback and resubmit if necessary. For now, this PR is rejected.

@PROMPTYLOL
Copy link

I have checked the png and have determined it is a legitimate image.

@PROMPTYLOL
Copy link

@Sitois

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants