Skip to content
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.

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

feat: Native Library patching and File Handle provider for arbitrary files #265

Closed
3 tasks done
Bluebotlabz opened this issue Dec 21, 2023 · 24 comments
Closed
3 tasks done
Assignees
Labels
Feature request Requesting a new feature that's not implemented yet

Comments

@Bluebotlabz
Copy link

Feature description

It would be extremely useful to support binary patching native libs as well as the ability to obtain a file handle to any file in the APK

This would be especially useful for patching more advanced applications, ie: Google Photos, or patching games, such as Unity games where patching the native libraries could allow for code changes whilst arbitrary file patching would let you modify asset bundles at ease

I think it would be important to provide a file handle rather than the traditional patched API as it may be important to include a custom parser or the ability to create and delete files as needed

Motivation

This feature should be implemented because it would allow a significant number of apps and games to be patchable through Revanced than otherwise and would allow for further changes to apps than otherwise, especially for games

Acknowledgements

  • This issue is not a duplicate of an existing feature request.
  • I have chosen an appropriate title.
  • All requested information has been provided properly.
@Bluebotlabz Bluebotlabz added the Feature request Requesting a new feature that's not implemented yet label Dec 21, 2023
@oSumAtrIX
Copy link
Member

oSumAtrIX commented Dec 22, 2023

In regards:

  • ReVanced Patcher MUST check if a library patch is involved and MUST read/write them JIT. This action is taken for efficiency reasons.

  • A new LibraryPatch MAY be added, and if implemented, the ReVanced Patcher MUST offer a LibraryContext : Context as a parameter. This context MUST allow reading and writing libraries in the /lib path. Reading is accomplished by opening a stream directly to the ZipEntry, and writing is done by opening a stream to the disk.

    context["/lib/some.lib"].let { file: File -> 
        // Can open an input stream to read from ZipEntry
        // Can open an output stream to write/cache to disk
    }

    Additionally, the ReVanced Patcher MUST record a list of written libraries in the writing stage and provide them with the patch result to the caller.

  • Operations that depend on each other SHOULD share read operations, but write operations MUST be locked. If a patch opens a write stream without closing it, and another patch depends on it, an exception MUST be raised when attempting to open a write stream. A patch can lock write operations for another patch if it implements the Closeable interface. Here's how it works:

    1. Patch A depends on Patch B.
    2. Patch B is executed and happens to implement Closeable.
    3. In the PatchB#close method, Patch B closes a write stream to a file.
    4. After Patch B executes, Patch A is executed and tries to write to the same file.
    5. This operation fails because Patch B still holds the lock on the write stream.
    6. Patch A raises an exception.
    7. Since no other patch depends on Patch B, the PatchB#close method is called.
    8. This action frees the lock, allowing other patches to write to that file.
  • A FilePatch MUST be added for arbitrary files, including libraries if NO LibraryPatch is added. However, NO write access to resources or dex files SHOULD be granted, as these are meant to be modified by other kinds of patches. Read access MUST be granted.

  • Due to similarities in behavior between resource patches and the requested patch, refactoring classes regarding OOP patterns MAY be necessary. Consideration MUST be given to OOP principles, and a common interface or abstract class MAY be introduced.

@Bluebotlabz
Copy link
Author

You mention a possibility that LibraryPatch is not implemented, how would patching libraries work in this case?

@oSumAtrIX
Copy link
Member

oSumAtrIX commented Dec 22, 2023

If no LibraryPatch is added, FilePatch will be able to read and write libraries:

  • A FilePatch MUST be added for arbitrary files, including libraries if NO LibraryPatch is added. However, NO write access to resources or dex files SHOULD be granted, as these are meant to be modified by other kinds of patches. Read access MUST be granted.

@Bluebotlabz
Copy link
Author

Bluebotlabz commented Dec 22, 2023

That's actually not a bad idea to be honest, and LibraryPatch will probably inherit from FilePatch, though long-term it would be interesting to expand upon it by adding convenience functions specific to libraries, such as locating a symbol in a library and such

@oSumAtrIX
Copy link
Member

oSumAtrIX commented Dec 22, 2023

A FilePatch MUST NOT be open for inheritance for a LibraryPatch as it already offers all implementations that a LibraryPatch would need.

@Bluebotlabz
Copy link
Author

Bluebotlabz commented Dec 22, 2023

Oh right that makes sense
It would just be a copy of it then (but with library specific checks ofc)

@Bluebotlabz
Copy link
Author

Bluebotlabz commented Jan 1, 2024

Couldn't I just implement this by mostly modifying ResourceContext and ResourcePatch and having it so that it just reads/writes to the streams directly rather than using DomFileEditor?
(or probably just an alternative to DomFileEditor that provides direct access to the streams?)

The whole closing mechanism is somewhat confusing to me tbh, it seems that it only actually writes out the file on a "close" and the file is otherwise held in memory?

@oSumAtrIX
Copy link
Member

It will be held in memory unless you do not close the stream. The reason for the closing mechanism is that two patches may attempt to write to the same file which can be a problem.

@Bluebotlabz
Copy link
Author

Fair, however, I can see this potentially being problematic when trying to patch large files/libraries

Especially stuff like Unity bundles which can exceed sizes in the gigabytes

@oSumAtrIX
Copy link
Member

The locking mechanism is currently only present when you use DomFileEditor. For input streams, no locking mechanism is needed. The locking process is only necessary when you open a write stream

@Bluebotlabz
Copy link
Author

Bluebotlabz commented Jan 1, 2024

Ah, that makes sense so the patch would just, for example, effectively store addresses and data to write at those addresses, then on the close operation, it would just modify the inputstream as it's being written to the outputstream

At the moment I'm mainly just looking into modifying DomFileEditor code as a FileEditor which will probs implement functions for writing and reading to/from certain addresses...

I do kind of feel like I'm overthinking this though

@oSumAtrIX
Copy link
Member

DomFileEditor was specifically made for XML and other kinds of DOM files. If you want to open file streams you can currently do so with context["file"].inputStream()

@Bluebotlabz
Copy link
Author

wait is that actually currently implemented or are you just proposing implementation?

@oSumAtrIX
Copy link
Member

This is implemented

@Bluebotlabz
Copy link
Author

oh lol
so strictly speaking, I don't actually need to implement anything for reading, I just need... something for writing

@Bluebotlabz
Copy link
Author

Also, is there any reason copying the general structure of the DomFileEditor but for writing general content to files is a bad idea?

@oSumAtrIX
Copy link
Member

It was primarily done for DomFileEditor as we would have to introduce custom write and read methods or properly implement all overloads for File if we were want to include the locking mechanism for arbitrary files as well.

@Bluebotlabz
Copy link
Author

Bluebotlabz commented Jan 1, 2024

Well didn't you mention that we would need the locking mechanism anyway?
Honestly my current plan just involves making something like DomFileEditor which stores the changes a patch wants to make, then applies them on close

Mostly for consistency and also because I'm lazy and it seems easy to do (famous last words)

@oSumAtrIX
Copy link
Member

Your context allows opening read and write streams only. This means that implementing the locking mechanism doesn't require the overhead mentioned in my previous comment as you'd only need to consider the two methods to open read and write streams. Feel free to refactor the code in a way that makes it suitable for arbitrary files as well, the current strategy to just support locking for DomFileEditor is slightly arbitrary and not really ideal.

@Bluebotlabz
Copy link
Author

Bluebotlabz commented Jan 1, 2024

Right... Couldn't I also have something like the current context["file"].inputStream() stuff but for outputStream and that would be fine?

To be honest I don't really know what I'm doing and I'm just trying to implement this by modifying existing code as much as possible...

@oSumAtrIX
Copy link
Member

context["file"] returns you a File, hence no locking mechanism.

@Bluebotlabz
Copy link
Author

Wait so can't I just use that to patch arbitrary files anyway?

@oSumAtrIX
Copy link
Member

As it tries to open a file in a directory, but the libraries are not present it would fail. You'd need to read it from the APK file as a stream for example and if you modify it, write back to disk. More particularly you can find the full procedure in my initial comment above

@Bluebotlabz
Copy link
Author

Oh right, that makes sense

To be honest, whilst it's somewhat jank, as an initial implementation I'll probably just make a modified version of the resource patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature request Requesting a new feature that's not implemented yet
Projects
Status: Done
Development

No branches or pull requests

2 participants