Skip to content

fix: prevent null pointer dereference by validating file object before caching check - #6723

Merged
swick merged 2 commits into
flatpak:mainfrom
IGS-GIT:main
Jul 3, 2026
Merged

swick merged 2 commits into
flatpak:mainfrom
IGS-GIT:main

Conversation

@IGS-GIT

@IGS-GIT IGS-GIT commented Jul 2, 2026 •

Copy link
Copy Markdown
Contributor

Commit c4fce9e added a check to fail early when file-forwarding is attempted with empty paths (fixing a crash in #6689). However, the check flatpak_file_get_path_cached (file) == NULL is too broad:

  • For empty paths, it returns NULL and correctly errors out.
  • For remote web URIs (like https://google.com inside @@U ... @@), the file object is never initialized (NULL), meaning it also evaluates to NULL here and causes Flatpak to throw an Invalid path error.
  • This regressed and broke all external link redirects from sandboxed applications (e.g., clicking login links or web links in Fractal or Element) to the host's default web browser.

This was fixed by ensuring we only fail on NULL caches paths if a local file object was created/recognized. URLs are now exempt from this check

Fixes: c4fce9e ("run: Error out if file forwarding of empty paths is attempted")

@swick

swick commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Do you mind adding some tests for this as well?

@swick

swick commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

CI doesn't seem happy about the tests.

The commit message should be limited to 80 chars per line. The prefixes should be tests and run (its not about what they are but what is being changed).

Let's also add Fixes: c4fce9e4 ("run: Error out if file forwarding of empty paths is attempted")

@IGS-GIT
IGS-GIT force-pushed the main branch 3 times, most recently from a18d3ce to 3962815 Compare July 2, 2026 18:07
@IGS-GIT

IGS-GIT commented Jul 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Yay the tests aren't blowing everything up anymore
yippee-3060024422

@swick

swick commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

LGTM, but the fixes tag is in the wrong commit, and we still have a >80 line.

IGS-GIT added 2 commits July 3, 2026 10:05
Verify that the file object is valid before checking its cached path
to avoid a potential NULL pointer dereference.

Fixes: c4fce9e ("run: Error out if file forwarding of empty paths is attempted")
Add regression tests to verify that remote URIs and local files can
be forwarded correctly, and that empty paths result in an error.
@IGS-GIT

IGS-GIT commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

LGTM, but the fixes tag is in the wrong commit, and we still have a >80 line.

Sorry got sidetracked with the test cases. I made the requested changes. Should be all good now

@swick
swick enabled auto-merge July 3, 2026 17:14
@swick

swick commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Thanks a lot!

@swick
swick added this pull request to the merge queue Jul 3, 2026
Merged via the queue into flatpak:main with commit f653896 Jul 3, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants