mirror of
https://github.com/flutter/flutter.git
synced 2026-02-20 02:29:02 +08:00
daadbba856
### Overview
Restricts the `--aot-shared-library-name` flag used for launching
`FlutterFragment`s to only accept paths that point to the application's
internal storage.
### Why restrict the path at all
Currently, when an application launches a `FlutterFragment` with an
`Intent` that passes the `--aot-shared-library-name` flag, the Flutter
engine [will call
`dlopen`](f45a8f8b71/engine/src/flutter/fml/platform/posix/native_library_posix.cc (L14))
on the native code that lives where the passed path points with no
checks. So, if that path points to malicious code, we've got a problem.
See go/flutter-fragment-flag-security-analysis for full context.
By restricting the path to a trusted location, we ensure that the
application developer is either writing their own native code (that we
can assume not to be malicious) or is importing third party code and
manually packaging it with their application code, ensuring that it has
been vetted by the application author (or at least indicating that it
should be vetted).
### Why restrict the path to the application's internal storage
The [native code directory within internal
storage](https://developer.android.com/reference/android/content/pm/ApplicationInfo#nativeLibraryDir)
is where native JNI libraries are stored by default when the AGP
[`useLegacyPackaging`
option](https://developer.android.com/build/releases/past-releases/agp-4-2-0-release-notes#compress-native-libs-dsl)
is set to true, so any compiled native code that's part of an Android
app will live there.
If `useLegacyPackaging` is set to false or the application developer
wishes to reference native code some other way, they can manually
package it within internal storage, which is generally a secure place to
store files for your application needs. Also, as far as I'm aware, this
is the only secure place the application developers could reference
native code *by path* in their application (for example, developers
could put their native code in the `app/src/main/assets` directory, then
copy it into internal storage for reference by path).
_My primary sources:_
https://developer.android.com/ndk/guides/concepts
https://developer.android.com/studio/projects/add-native-code
https://developer.android.com/training/data-storage
### Prior art
Flutter has enforced a similar sort of path restriction for the
`file_selector_android` package; see
https://github.com/flutter/packages/pull/8184. Just like that PR does,
this PR makes sure to canonicalize paths passed to the flag and attempts
to test sneaky partial internal storage paths to mitigate path traversal
attacks.
### Future work
As mentioned in my TODO, the pattern of willy-nilly adding any flags
passed in an `Intent` to launch a `FlutterFragment` is dangerous. We
plan to address this vulnerability in a future PR:
https://github.com/flutter/flutter/issues/172553
### P.S. This PR also renames `application_library_path` to
`application_library_paths`
This is actually a vector of AOT shared library (plural) paths that can
include the one passed with the `--aot-shared-library-name` flag as well
as the fallbacks.
## Pre-launch Checklist
- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Flutter Engine
Setting up the Engine development environment
See here
gclient bootstrap
Flutter engine uses gclient to manage dependencies.
If you've already cloned the flutter repository: