flathub / com.adobe.Flash-Player-Projector

https://flathub.org/apps/details/com.adobe.Flash-Player-Projector
9 stars 9 forks source link

Configuration persistance hack does not work for Local Shared Objects #41

Closed sm-Fifteen closed 4 years ago

sm-Fifteen commented 4 years ago

This flatpak of the flash projector is unable to correctly persist Local Shared Objects (highscores, save files, etc.). Those files are normally persisted in a sub-directory of ~/.macromedia/Flash_Player/#SharedObjects with a randomly generated name for what is described as security purposes. If one directory already exists, it seems like Flash Player will just take the first one available.

Under the current release, this appears to be broken due to a bug with fix-config-location.c. From what I can tell, all it does is override various FS-related syscalls to do the equivalent of --persist=.macromedia and --persist=.adobe. It doesn't work here because Flash Player uses lstat to check for the permissions of the directories inside ~/.macromedia/Flash_Player/#SharedObjects, which is not covered by the hack and causes the player to create a new Shared Object directory every time it is launched.

strace of the issue:

Scans for subdirectories:

stat("/home/user/.var/app/com.adobe.Flash-Player-Projector/config/.macromedia/Flash_Player/#SharedObjects", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
openat(AT_FDCWD, "/home/user/.var/app/com.adobe.Flash-Player-Projector/config/.macromedia/Flash_Player/#SharedObjects", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 16
fstat(16, {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
getdents64(16, /* 17 entries */, 32768) = 528

Here is the bug:

lstat("/home/user/.macromedia/Flash_Player/#SharedObjects/PH4H2M4C", 0x7fff58e219c0) = -1 ENOENT (Aucun fichier ou dossier de ce type)
lstat("/home/user/.macromedia/Flash_Player/#SharedObjects/L5HUP3XR", 0x7fff58e219f0) = -1 ENOENT (Aucun fichier ou dossier de ce type)
close(16)                               = 0

Attempts to create a new directory instead

openat(AT_FDCWD, "/dev/urandom", O_RDONLY) = 16
fstat(16, {st_mode=S_IFCHR|0666, st_rdev=makedev(0x1, 0x9), ...}) = 0
ioctl(16, TCGETS, 0x7fff58e21880)       = -1 EINVAL (Argument invalide)
read(16, "\275H\250\216\227\372\211\343\254\275\226c\304\24\323\27\214P\244\321\236\337*\306J<\376\222Hc\205\313"..., 4096) = 4096
close(16)                               = 0
stat("/home/user/.var/app/com.adobe.Flash-Player-Projector/config/.macromedia/Flash_Player/#SharedObjects/7JJQZ4KD", 0x7fff58e219f0) = -1 ENOENT (Aucun fichier ou dossier de ce type)
mkdir("/home/user/.var/app/com.adobe.Flash-Player-Projector/config/.macromedia/Flash_Player/#SharedObjects/7JJQZ4KD", 0700) = 0
stat("/home/user/.var/app/com.adobe.Flash-Player-Projector/config/.macromedia/Flash_Player/#SharedObjects/7JJQZ4KD/macromedia.com/support/flashplayer/sys/settings.sol", 0x7fff58e21b70) = -1 ENOENT (Aucun fichier ou dossier de ce type)

I'm not entirely sure why this hack is required in the first place. Why not simply use --persist=.macromedia --persist=.adobe or --filesystem=~/.macromedia/Flash_Player:create?

sm-Fifteen commented 4 years ago

Having now looked at this more in-depth, this issue is really just an extension of the way opening SWF files is handled, because the projector needs to show the user a file picker that lets them open more or less any SWF file.

Having --filesystem=host:ro (as is currently the case) works, but is incompatible with --persist which only works if the user's real home directory is unmounted, thus requiring the hack. host is arguably a far too powerful of a permission to give to something like Flash Player (which apparently still attempts to read files from ~/.mozilla), even in read-only.

Applications are advised to use GtkFileChooserNative to let containerized applications access any file from outside the sandbox so long at the user explicitly requests it, but the standalone Flash Player predates this (or GTK3, even) and obviously can't use it, nor can we easily shim it like we can with syscalls.

With that said, the filesystem permissions could probably be reduced to a couple of XDG user data directories, combined with something like a flatpak run --file-forwarding com.adobe.Flash-Player-Projector @@ %f @@ shortcut in the desktop entry to let users open the files directly from the system file browser.

--filesystem=~/.macromedia/Flash_Player:create would work and avoid any surprises for users attempting to manipulate the content of the #SharedObjects directory by themselves (which some might be tempted to do once Flash stops working on browsers so they don't lose past data), and there's no risk of breaking the save data that was already residing in .var/app/com.adobe.FlashPlayer because those have seemingly never worked with the current Flatpak permissions.

hadess commented 4 years ago

The "hack" is needed to avoid polluting the user's home directory. Help root-causing why it tries to create files in directories it never created in the first place would be useful.

sm-Fifteen commented 4 years ago

The "hack" is needed to avoid polluting the user's home directory.

I'm just calling it a "hack" because using ld_preload and a shim to do what should arguably be done with --persist or some other form of sandbox configuration is not exactly clean and generally less reliable than the alternative. Even the commit that introduced this calls it "an LD_PRELOAD hack".

Help root-causing why it tries to create files in directories it never created in the first place would be useful.

From my initial report, it seems to be because the shim doesn't cover one of the syscalls Flash is trying to use, either lstat or the getdents64 just before it in the strace log.

hadess commented 4 years ago

From my initial report, it seems to be because the shim doesn't cover one of the syscalls Flash is trying to use, either lstat or the getdents64 just before it in the strace log.

Right, so lstat probably needs wrapping, but not getdents64, but rather open when O_DIRECTORY is used.

hadess commented 4 years ago

A test version should be available shortly in https://github.com/flathub/com.adobe.Flash-Player-Projector/pull/42 if you want to test it.

sm-Fifteen commented 4 years ago

I'm not too experienced with running test builds of flatpak applications, so I might be doing it wrong, but that doesn't seem to have solved it:

$ flatpak install --user https://dl.flathub.org/build-repo/22420/com.adobe.Flash-Player-Projector.flatpakref
$ flatpak run --branch=test --command=sh --devel com.adobe.Flash-Player-Projector
[📦 com.adobe.Flash-Player-Projector ~]$ strace flashplayer |& grep -C5 ~/.macromedia/Flash_Player/#SharedObjects
mmap(0x3a7765d40000, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x3a7765d40000
stat("/home/user/.var/app/com.adobe.Flash-Player-Projector/config/.macromedia/Flash_Player/#SharedObjects", {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
openat(AT_FDCWD, "/home/user/.var/app/com.adobe.Flash-Player-Projector/config/.macromedia/Flash_Player/#SharedObjects", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = 16
fstat(16, {st_mode=S_IFDIR|0700, st_size=4096, ...}) = 0
getdents64(16, /* 38 entries */, 32768) = 1200
lstat("/home/user/.macromedia/Flash_Player/#SharedObjects/5YH6L8PZ", 0x7ffd5a2ba9f0) = -1 ENOENT (Aucun fichier ou dossier de ce type)
lstat("/home/user/.macromedia/Flash_Player/#SharedObjects/PH4H2M4C", 0x7ffd5a2baa20) = -1 ENOENT (Aucun fichier ou dossier de ce type)
close(16)                               = 0
openat(AT_FDCWD, "/dev/urandom", O_RDONLY) = 16
fstat(16, {st_mode=S_IFCHR|0666, st_rdev=makedev(0x1, 0x9), ...}) = 0
ioctl(16, TCGETS, 0x7ffd5a2ba8b0)       = -1 EINVAL (Argument invalide)
read(16, "\225&\313\326\343\275\316g\210b\22=\276P\202.\326\377\2001\370\275\371Y+s\4:KTqm"..., 4096) = 4096

Changing the grepped path to ~/.macromedia/ or ~/.adobe also reveals there's a few more syscalls running with uncorrected paths.

lstat("/home/user/.adobe/Flash_Player/AssetCache/TPAY2VGE", 0x7ffe8bc186f0) = -1 ENOENT (Aucun fichier ou dossier de ce type)
lstat("/home/user/.adobe/Flash_Player/AssetCache/F6FUDUHQ", 0x7ffe8bc18720) = -1 ENOENT (Aucun fichier ou dossier de ce type)

lstat("/home/user/.macromedia/Flash_Player/#SharedObjects/5YH6L8PZ", 0x7ffe765bcb20) = -1 ENOENT (Aucun fichier ou dossier de ce type)
lstat("/home/user/.macromedia/Flash_Player/#SharedObjects/PH4H2M4C", 0x7ffe765bcb50) = -1 ENOENT (Aucun fichier ou dossier de ce type)
truncate("/home/user/.macromedia/Flash_Player/macromedia.com/support/flashplayer/sys/settings.sxx", 0) = -1 ENOENT (Aucun fichier ou dossier de ce type)
unlink("/home/user/.macromedia/Flash_Player/macromedia.com/support/flashplayer/sys/settings.sol") = -1 EROFS (Système de fichiers accessible en lecture seulement)
rename("/home/user/.macromedia/Flash_Player/macromedia.com/support/flashplayer/sys/settings.sxx", "/home/user/.macromedia/Flash_Player/macromedia.com/support/flashplayer/sys/settings.sol") = -1 EROFS (Système de fichiers accessible en lecture seulement)

EDIT: Those last 3 would probably explain why adjusting the settings (like disabling video acceleration, which otherwise creates strange video glitches with my Nvidia GPU) don't seem to do anything either.