Post-rewriter changes #1
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Mostly build script changes, with some minor source code changes.
Minus changes to the libjpeg source files.
This effectively reverts unwanted rewrites to files outside of the rewrite set.
There were C and C++ functions with the same name, resulting in duplicated IA2_DEFINE_WRAPPER invocations. Renaming the C++ functions to be different fixes the issue.
Also tweak how we're dlopening libxul.so
|
(Automated Close) Please do not file pull requests here, see https://firefox-source-docs.mozilla.org/contributing/how_to_submit_a_patch.html |
These were taking precedence over the overrides defined in partition-alloc, preventing allocations from going through PA. Disabling this gets more allocations to go through PA.
…om system lib In a few places ff depends on system libraries (e.g. libfontconfig) that internally use malloc, and then ff uses free on those allocations. Since ff doesn't build these system libs, we can't force them to go through PA. This means that when ff frees the allocation it has to use vanilla free, otherwise PA doesn't recognize the allocation and crashes.
As noted in cpsysroot.sh, we have a mismatch between the version of libc that is used to build partition-alloc and the version in Firefox's custom sysroot. This results in linker errors because Firefox's libc doesn't have all of the symbols that PA requires. To work around this, we copy the necessary system libraries into the sysroot. This also required some minor header changes. In some cases, the build system tests for the presence of libc symbols and sets preprocessor defines based on the presence of some symbols. In ptthread.c it attempts to use gettid, which isn't present in the default sysroot but is present in the newer system library. This causes a mismatch because even though the symbol is present in the updated libc, we're still using the old libc headers which do not define that symbol. To work around this, I've modified the header to always define the custom gettid that is used normally. A similar situation occurs in `event-config.h`, where the build system checks for the presence of some arc4random functions. The symbols are present, so the build system sets corresponding defines, but then the headers do not define the functions. I have hard coded it to assume that the symbols are not present so that it uses the shim arc4random library that it normally uses when we don't modify the sysroot.
There are a couple of build settings that we need to specify in order for Firefox to build correctly. Normally this wouldn't be committed, but we want to preserve this so that we can still build correctly in a fresh environment.
Data inside of the `nsJPEGDecoder` object gets passed into libjpeg, so the decoder needs to be on the shared heap to avoid compartment violations.
Firefox sets some environment variables at runtime, which means some of the env vars point to compartment 1's heap. libjpeg then tries to read env vars, causing a compartment violation when it gets the vars that were set at runtime. To fix this we copy any strings passed to `setenv` and `putenv` to the shared heap before setting the env var. This leaks some memory, but probably not much.
These are the remaining allocations that libjpeg needs access to. One minor note: We change `SourceBuffer::Chunk` and `DownscalingFilter` to always allocate their data on the shared heap, but I'm pretty sure these are used for other decoding purposes beyond just libjpeg. Doing it this way is potentially a security hole if libjpeg somehow gets control of a buffer for a different decoding process. A more robust solution might be to only have chunks/filters that are used with libjpeg be allocated on the shared heap, but that's likely a more involved change that isn't worth doing for the purpose of the Firefox demo.
For a minimal demonstration of IA2, we can simply change a `shared_malloc` back to a `malloc` and then run ff. This will at least demonstrate that IA2 is correctly preventing libjpeg from touching data outside its compartment.
It's just a jpeg with a comment marker in it, which we can use as the trigger for our synthetic exploit.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.