Skip to content

Verify downloaded binaries #4275

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.

Sign up for GitHub

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

Jump to bottom
Open
YoshiRulz opened this issue Mar 18, 2025 · 3 comments · May be fixed by #4287
Open

Verify downloaded binaries #4275

YoshiRulz opened this issue Mar 18, 2025 · 3 comments · May be fixed by #4287
Labels
App: EmuHawk Relating to EmuHawk frontend re: RetroAchievements Request: Feature/Enhancement For feature requests or possible improvements
Milestone
2.10.1

Comments

@YoshiRulz
Copy link
Member

YoshiRulz commented Mar 18, 2025 •
edited
Loading

BizHawk/src/BizHawk.Client.EmuHawk/AVOut/FFmpegDownloaderForm.cs

Lines 83 to 104 in 0e9c21e

//try acquiring file
using (var hf = new HawkFile(fn))
{
using (var exe = OSTailoredCode.IsUnixHost ? hf.BindArchiveMember("ffmpeg") : hf.BindFirstOf(".exe"))
{
//last chance. exiting, don't dump the new ffmpeg file
if (exiting)
return;
exe!.GetStream().CopyTo(fs);
fs.Dispose();
if (OSTailoredCode.IsUnixHost)
{
OSTailoredCode.ConstructSubshell("chmod", $"+x {FFmpegService.FFmpegPath}", checkStdout: false).Start();
Thread.Sleep(50); // Linux I/O flush idk
}
}
}
//make sure it worked
if (!FFmpegService.QueryServiceAvailable()) throw new Exception("download failed");
succeeded = true;

BizHawk/src/BizHawk.Common/FFmpegService.cs

Lines 16 to 18 in 0e9c21e

private const string BIN_HOST_URI_LINUX_X64 = "https://github.com/TASEmulators/ffmpeg-binaries/raw/master/ffmpeg-4.4.1-static-linux-x64.7z";
private const string BIN_HOST_URI_WIN_X64 = "https://github.com/TASEmulators/ffmpeg-binaries/raw/master/ffmpeg-4.4.1-static-windows-x64.7z";

Not great, there's some trustworthiness to those addresses, but it gets worse...

BizHawk/src/BizHawk.Client.EmuHawk/RetroAchievements/RAIntegration.Update.cs

Lines 47 to 80 in 0e9c21e

private static bool DownloadDll(string url)
{
if (url.StartsWithOrdinal("http:"))
{
// force https
url = url.Replace("http:", "https:");
}
using var downloadForm = new RAIntegrationDownloaderForm(url);
downloadForm.ShowDialog();
return downloadForm.DownloadSucceeded();
}
public static bool CheckUpdateRA(IDialogParent dialogParent)
{
try
{
var http = new HttpCommunication(null, "https://retroachievements.org/dorequest.php?r=latestintegration", null);
var info = JsonConvert.DeserializeObject<Dictionary<string, object>>(http.ExecGet());
if (info.TryGetValue("Success", out var success) && (bool)success)
{
var lastestVer = new Version((string)info["LatestVersion"]);
var minVer = new Version((string)info["MinimumVersion"]);
if (_version < minVer)
{
if (!dialogParent.ModalMessageBox2(
text:
"An update is required to use RetroAchievements. Do you want to download the update now?",
caption: "Update",
icon: EMsgBoxIcon.Question,
useOKCancel: false)) return false;
DetachDll();
var ret = DownloadDll((string)info["LatestVersionUrlX64"]);

...because this blindly follows any address returned in the first response 🙃 And bonus points for copy-pasting:

BizHawk/src/BizHawk.Client.EmuHawk/RetroAchievements/RAIntegrationDownloaderForm.cs

Lines 94 to 109 in 0e9c21e

//try acquiring file
using (var dll = new HawkFile(fn))
{
var data = dll!.ReadAllBytes();
//last chance. exiting, don't dump the new RAIntegration file
if (_exiting)
return;
DirectoryInfo parentDir = new(Path.GetDirectoryName(_path)!);
if (!parentDir.Exists) parentDir.Create();
if (File.Exists(_path)) File.Delete(_path);
File.WriteAllBytes(_path, data);
}
_succeeded = true;

Simple solution for FFmpeg is to hardcode the checksum.

RA's API seems to be down at the moment so I can't check if that includes a checksum, but even if it did, it would need to be signed as well (and we hardcode the pubkey).
And assert info["LatestVersionUrlX64"].StartsWith("https://retroachievements.org/").
@YoshiRulz YoshiRulz added App: EmuHawk Relating to EmuHawk frontend re: RetroAchievements Request: Feature/Enhancement For feature requests or possible improvements labels Mar 18, 2025
@YoshiRulz YoshiRulz added this to the 2.10.1 milestone Mar 18, 2025
@CasualPokePlayer
Copy link
Member

FYI the automatic updating of RAIntegration dll only occurs if the user manually puts in an RAIntegration dll in in the first place (either manually downloading it or it coming from a different RA emulator), it doesn't affect normal users (mainly just achievement developers).

@Morilli
Copy link
Collaborator

Morilli commented Mar 18, 2025

I don't really see the issue with the ffmpeg download as

  1. It's github.com
  2. It's the TASEmulators repo

Of course we could check the checksum because we know it, but I wouldn't call this problematic per se.

@CasualPokePlayer
Copy link
Member

Also, the user is told the exact url which the download will be occurring on. If the user does not trust that URL, they can exit out of the form, and no download will take place.

YoshiRulz added a commit that referenced this issue Apr 10, 2025
@YoshiRulz
Add some more validation to {FFmpeg,RAIntegration}DownloaderForm
75a10e1 
see #4275
YoshiRulz added a commit that referenced this issue Apr 10, 2025
@YoshiRulz
Add some more validation to {FFmpeg,RAIntegration}DownloaderForm
1983966 
see #4275
@YoshiRulz YoshiRulz linked a pull request Apr 10, 2025 that will close this issue
Open
@YoshiRulz YoshiRulz linked a pull request Apr 10, 2025 that will close this issue
Downloader dedup #4287
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
App: EmuHawk Relating to EmuHawk frontend re: RetroAchievements Request: Feature/Enhancement For feature requests or possible improvements
Projects
None yet
Milestone
2.10.1
Development

Successfully merging a pull request may close this issue.

Downloader dedup TASEmulators/BizHawk
3 participants
@YoshiRulz @Morilli @CasualPokePlayer