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

py shortcut icon still broken #1001

Closed
Ana06 opened this issue Apr 19, 2024 · 4 comments
Closed

py shortcut icon still broken #1001

Ana06 opened this issue Apr 19, 2024 · 4 comments
Assignees
Labels
🐛 bug Something isn't working 🌀 FLARE-VM A package or feature to be used by FLARE-VM

Comments

@Ana06
Copy link
Member

Ana06 commented Apr 19, 2024

Details

I have just installed ida.diaphora.vm 3.2.0 and the icon of the .py file looks broken:

image

@emtuls hadn't this been addressed in #675?

We are planing to change how diaphora is installed (without shortcut): #994, so this issue is about ensuring that the .py shortcut look nice and not about fixing diaphora specifically. But I noticed the issue while testing diaphora.

@Ana06 Ana06 added 🐛 bug Something isn't working 🌀 FLARE-VM A package or feature to be used by FLARE-VM labels Apr 19, 2024
@emtuls
Copy link
Member

emtuls commented Apr 23, 2024

Strange...I just tried it on a fresh setup and it looks fine for me? 🤔
image

@emtuls
Copy link
Member

emtuls commented Apr 25, 2024

Ah, so I was mistaken @Ana06. It appeared to work because of my vscode I had installed.

So, this issue is because we opted to go the route of requiring tools that were python based to use python as the executablePath (which will make python the Target for the shortcut), which will in turn make python the icon due to iconLocation being set to executablePath: https://github.com/mandiant/VM-Packages/blob/main/packages/common.vm/tools/vm.common/vm.common.psm1#L228

The downside that this had was that we don't default all Python files to use the Python icon as a shortcut if they are something like ida.diaphora, since it's just a python plugin and we weren't planning to execute it directly with Python.

The other downside is that we have to call VM-Install-Shortcut directly, like such: https://github.com/mandiant/VM-Packages/blob/main/packages/didier-stevens-beta.vm/tools/chocolateyinstall.ps1#L22-L25

An alternative we could do, that I think I may have suggested somewhere, would be to add inside of VM-Install-From-Zip, a check for if the $executableName has an extension of .py, and then we could do:

if ([System.IO.Path]::GetExtension($executableName) -eq ".py") {
    $executablePath = (Get-Command python).Source
    $filePath = Join-Path $toolDir "$toolName.py"
    $arguments = $filePath + $arguments
    $consoleApp = $true
    VM-Install-Shortcut -toolName $toolName -category $category -executablePath $executablePath -consoleApp $consoleApp -arguments $arguments
}

I've just tested that and it would work, if you want to go that route. It will also require adding python3.vm to ida.diaphora.vm since the shortcut relies on having Python existing on the system.

@Ana06
Copy link
Member Author

Ana06 commented Apr 25, 2024

Thanks for all the research @emtuls! I do not like the idea of complicating VM-Install-From-Zip just because of one package, specially because diaphora is a very special package (and IDA plugin which we are not installing as a plugin). I think we should focus on #994 and close this issue.

@emtuls
Copy link
Member

emtuls commented Apr 25, 2024

Sounds good!

Just a heads up, if I'm not mistaken, if we start to get more python tools, we will be forced to use VM-Install-Shortcut directly unless we choose to do something like #1011 in the future. 🙂

@emtuls emtuls closed this as completed Apr 25, 2024
@Ana06 Ana06 added this to the FLARE-VM 2024 Q2 milestone Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🌀 FLARE-VM A package or feature to be used by FLARE-VM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants