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

Running app in C:\Windows\System32 gives the error rundll32 resolves to executable in current directory (.\rundll32.exe) #3342

Open
2 tasks done
williambrode opened this issue Oct 18, 2022 · 13 comments
Labels
bug Something isn't working Hacktoberfest Issues that count towards Hacktoberfest scores.

Comments

@williambrode
Copy link
Contributor

Checklist

  • I have searched the issue tracker for open issues that relate to the same problem, before opening a new one.
  • This issue only relates to a single bug. I will open new issues for any other problems.

Describe the bug

Running app in C:\Windows\System32 gives the error rundll32 resolves to executable in current directory (.\rundll32.exe)

The code calls fyne.CurrentApp().OpenURL(url)

Which hits this code (windows):

func (a *fyneApp) OpenURL(url *url.URL) error {
	cmd := a.exec("rundll32", "url.dll,FileProtocolHandler", url.String())
	cmd.Stdin, cmd.Stdout, cmd.Stderr = os.Stdin, os.Stdout, os.Stderr
	return cmd.Run()
}

But with the go 1.19 changes to PATH lookups this fails. The same error will happen for all other fyne calls to "rundll32". I assume the fix is to reference the full path to it: "C:\Windows\System32\rundll32"

See https://go.dev/doc/go1.19 - PATH lookups for details.

How to reproduce

  1. Create fyne app with golang 1.19 on Windows that calls fyne.CurrentApp().OpenURL(url)
  2. Run the app with the working directory as C:\Windows\System32
  3. Notice the error.

Screenshots

No response

Example code

fyne.CurrentApp().OpenURL(url)

Fyne version

2.2.3

Go compiler version

1.19.1

Operating system

Windows

Operating system version

Windows 10

Additional Information

No response

@williambrode williambrode added the unverified A bug that has been reported but not verified label Oct 18, 2022
@williambrode
Copy link
Contributor Author

@Bluebugs
Copy link
Contributor

If that solution work, it shouldn't be difficult to get it fixed. Did you try that change locally?

@andydotxyz andydotxyz added bug Something isn't working Hacktoberfest Issues that count towards Hacktoberfest scores. and removed unverified A bug that has been reported but not verified labels Oct 18, 2022
@williambrode
Copy link
Contributor Author

I couldn't reproduce it in the debugger because its been fixed: golang/go#53536

I had forgotten that fyne-cross is what actually builds the app so its not using golang 1.19.1 (even though I'm using that for testing). So the go version the issue appears on would be 1.18.3 or 1.18.4 I believe. I'm on fyne-cross@v1.3.0 .

@Bluebugs
Copy link
Contributor

Ah, I see. I think our template is missing asking about fyne-cross. @lucor , @Jacalz it seems we might want to update to go 1.19.1 for next major release of fyne-cross. Do you guys have any opinion on the subject?

@Jacalz
Copy link
Member

Jacalz commented Oct 22, 2022

Yes, we should probably update fyne-cross to use Go 1.19 quite soon. Do I understand this correctly as being a bug in the Go compiler (or standard library) and not one of ours? If so, we should probably close it.

@williambrode
Copy link
Contributor Author

I don't think its easy to place blame on this one. I think fyne should be fixed even though go has helped fix the issue since people will still try to use fyne on a lower version of go.

@Jacalz
Copy link
Member

Jacalz commented Oct 22, 2022

Ah, I was confused with the information in the ticket and the error message; especially the issue saying that it is an issue in Go 1.19 but the Go bug report mention it being fixed in Go 1.19. I believe that the correct fix is to use execabs and not to reference the full path.

@Jacalz
Copy link
Member

Jacalz commented Oct 22, 2022

Looking at the code, it seems like we actually are using execabs already and that I was wrong. It is very strange that they did not backport that fix fix to older Go releases.

@williambrode
Copy link
Contributor Author

I linked the fix from another open source lib above. I don't know if they could have backported the fix or not. I think it was introduced in 18.x sometime.

@williambrode
Copy link
Contributor Author

Any chance we can either update fyne-cross or fix the issue directly pretty soon so I can fix it in my app? I'll create a PR if you will accept the solution from this reference: https://github.com/skratchdot/open-golang/blob/eef8423979666925a58eb77f9db583e54320d5a4/open/exec_windows.go#L15

@Bluebugs
Copy link
Contributor

We are working to do a minor release of fyne this week that will be followed by an update in fyne-cross. That should address your problem as I would prefer to not add workaround in fyne code base as much as possible.

@williambrode
Copy link
Contributor Author

Okay, I guess I see the fyne fix as both addressing a security vulnerability directly (since right now on older go versions a malicious rundll32 could be run), and fixing compatibility with go 18.x.

I should have mentioned too that the app is run by default in C:\Windows\System32 when run via the windows search bar. That's how we discovered the issue.

My issue will be fixed by either change, but I just wanted to make clear why I suggested fixing both.

@Bluebugs
Copy link
Contributor

Oh, hum, I kind of see how this could be used in a nefarious way. Make me wonder now if we shouldn't actually have a workaround specifically for 1.18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Hacktoberfest Issues that count towards Hacktoberfest scores.
Projects
None yet
Development

No branches or pull requests

4 participants