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

Magisk UDS(Unix Domain Socket) detection added #88

Closed
wants to merge 16 commits into from

Conversation

KimChangYoun
Copy link

Detects roots by searching for any 32-digit Unix Domain Socket name used by the Magisk daemon.
I would appreciate your review.
Thank you.

Detects roots by searching for any 32-digit Unix Domain Socket name used by the Magisk daemon.
Copy link
Owner

@scottyab scottyab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this new check looks very exciting. Could you expand a bit more about UDS, is that Unified Diagnostic Services? I'm interested to how the check works.

Couple of minor PR comments. But still need to test on Magisk device before can 👍 .

rootbeerlib/src/main/jni/toolChecker.cpp Outdated Show resolved Hide resolved
rootbeerlib/build.gradle Outdated Show resolved Hide resolved
@KimChangYoun
Copy link
Author

uds stands for unix domain socket, which is used by the Magisk daemon. The broken font is Korean and is an output string for debugging. I will soon clean up the code and commit again. Thanks for your quick feedback.

Add a description of the Magisk detection function
Changed debugging string in Korean to English
@KimChangYoun KimChangYoun changed the title Magisk UDS detection added Magisk UDS(Unix Domain Socket) detection added Dec 5, 2018
Changed buildToolsVersion to buildToolsVer.
However, we recommend that you change this value in the latest Android studio.
@KimChangYoun
Copy link
Author

KimChangYoun commented Dec 6, 2018

This method can detect the latest Magisk.
I also tested it on the Pixel 2 smartphone (Android 9.0) and found that it is possible to query the unix domain socket.

As a result of my checking, this method is detectable even when Magisk Hide is fully applied.
And I added Magisk UDS detection logic to the main UI.

@Stoisss
Copy link

Stoisss commented Jan 2, 2019

Are there any updates on this pull request as Magisk is gaining popularity?

@KimChangYoun
Copy link
Author

Are there any updates on this pull request as Magisk is gaining popularity?

It seems that Magisk is still being tested on the device.

Of course there is a possibility of false positives, but I would like to emphasize that Magisk and Magisk Hide are detectable.

This PR has not yet been applied to RootBeer, but you can download a version of RootBeer with Magisk detection from my Github.
https://github.com/KimChangYoun/rootbeer/releases

@yavski
Copy link

yavski commented Jan 25, 2019

Hi @scottyab, how are you? I'm working with @Stoisss on one app, and we've decided to use your lib as a means for root detection. Is there any news on this PR as it holds us back with one feature?

@comomind
Copy link

Hi, @KimChangYoun
Thanks for sharing your detection idea, it's pretty impressive for me. :)

Your detection method is based on USD length equal 32. And i'm just wondering why?

And I briefly read magisk source code & realized that magisk uses 2 (or more) UDS and it uses below initial value.

#define MAIN_SOCKET "d30138f2310a9fb9c54a3e0c21f58591"
#define LOG_SOCKET "5864cd77f2f8c59b3882e2d35dbf51e4"
https://github.com/topjohnwu/Magisk/blob/master/native/jni/include/magisk.h

After initialization phase they change their UDS name randomly (but still same length 32).

Q.1) Did I understand your method correctly?
Q.2) If so i think we need other solution to avoid false positive case, is there any good idea?

  • In this case false positive means detect normal application (it uses 32 length UDS) as rooted

@KimChangYoun
Copy link
Author

Dear @comomind

Thank you for your interest in my ideas.
Magisk Hide patches the original 32-character Unix Domain Socket name with a random 32-character string.

This is where you will find more information about Magisk Hide's source code. (there is a function with a name beginning with "patch")

Now, the answer to Q1.
Yes. You have understood exactly.

Answer to Q2.
Of course I have a better alternative,
However, I am currently targeting this detection to be adopted by RootBeer. :-)

We now take advantage of the stat command for Magisk detection.
@KimChangYoun
Copy link
Author

@comomind Here is the breakthrough Magisk 18 and Magisk Hide detection.
https://github.com/KimChangYoun/rootbeer/releases/tag/0.0.9

@comomind
Copy link

comomind commented Feb 1, 2019

Thanks for your new code :)

I briefly review your code. And I found that your suggestion is a combination of "checking UDS length" & "checking file existence".

I tested your code in my nexus 6 with magisk hide & magisk manager hide enabled.
You added some file existence check logic and that's not working properly.
Paths you added are not visible in application, stat() function failed always.

Q1. My test is failed, is there something wrong?
Q2. rootbeer already has magisk file check logic. Is there are difference with this?

  • checkForMagiskBinary()

Q3. checkFileStat() should be outside of loop, is there some reason?

magisk_file_detect_count += checkFileStat("/dev/.magisk.unblock");

@KimChangYoun
Copy link
Author

@comomind
Q1. Check the prototype declaration of the extern stat function.
Q2. checkForMagiskBinary() function is a function that the existing rootbeer used to detect Magisk, not a function I created. Of course, Magisk Hide does not detect it.
Q3. There is no other reason. checkFileStat() function can be called anywhere.

@KimChangYoun
Copy link
Author

I think it is right to develop it as a separate app, and I will close the PR.
My repository has been changed to rootbeerFresh to inherit the rootbeer.
See https://github.com/KimChangYoun/rootbeerFresh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants