Skip to content

Improve : shell/src/main/cpp/dpt_risk.cpp#139

Open
Cynthia-cnn wants to merge 4 commits intoluoyesiqiu:mainfrom
Cynthia-cnn:main
Open

Improve : shell/src/main/cpp/dpt_risk.cpp#139
Cynthia-cnn wants to merge 4 commits intoluoyesiqiu:mainfrom
Cynthia-cnn:main

Conversation

@Cynthia-cnn
Copy link
Copy Markdown

fix and add method protection

Comment thread shell/src/main/cpp/dpt_risk.cpp Outdated
long fd = syscall(__NR_openat, (long)AT_FDCWD, (long)"/proc/self/maps", (long)O_RDONLY, (long)0);
if (fd < 0) return false;

char buf[4096];
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The size here is fixed at 4096 bytes, which may prevent reading the entire file completely.

Comment thread shell/src/main/cpp/dpt_risk.cpp Outdated
syscall(__NR_close, (long)fd);
if (n > 0) {
buf[n] = '\0';
char* p = strstr(buf, "TracerPid:");
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It is better to use dpt_strstr instead of strstr.

Comment thread shell/src/main/cpp/dpt_risk.cpp Outdated
"pop %rbp\t\n"
#endif
);
asm volatile("brk #0" ::: "memory");
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This will expose the location of the crash and can be easily bypassed.

Copy link
Copy Markdown
Owner

@luoyesiqiu luoyesiqiu left a comment

Choose a reason for hiding this comment

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

There are some parts in the code that need to be modified.

@Cynthia-cnn
Copy link
Copy Markdown
Author

@luoyesiqiu Thanks for the review!
I've addressed all three points:

  • Increased "/proc/self/maps" buffer to "16Kb" and read fully to avoid truncation
  • Replaced all "strstr" with "dpt_strstr"
  • Kept crash logic centralized for now (can be inlined later per your suggestion)
    Ready for re-review...

@luoyesiqiu
Copy link
Copy Markdown
Owner

  • Kept crash logic centralized for now (can be inlined later per your suggestion)

I tried it out, and when debugging is detected, the crash location still gets exposed. My original implementation was intended to prevent attackers from knowing where the crash occurs.

@Cynthia-cnn
Copy link
Copy Markdown
Author

@luoyesiqiu You're absolutely right.

having a centralized dpt_crash() exposes the crash location and makes patching trivial.

I’ll remove the dpt_crash() function entirely and replace each call with inline, architecture-specific illegal instructions (e.g., brk #0 on ARM64) directly at the detection sites. This way, there’s no single symbol to hook, and crashes appear to originate from multiple unpredictable locations.

I’ll push the updated version shortly.

Because a declared crash symbol already constitutes a commitment point.
Even without being called, it exposes intent and becomes a trivial anchor for patching or symbol-based analysis.
@Cynthia-cnn
Copy link
Copy Markdown
Author

@luoyesiqiu

In the updated version there is no explicit if (detected) crash nor a centralized commit point.

All detection signals only perturb an entropy value (e) through arithmetic mixing. The control flow never branches on a detection result.

Whether execution remains valid or collapses is an emergent consequence of the transformed execution state, not a decision.

There is no single function that represents “the crash”; corruption is distributed and non-deterministic, and failure manifests naturally via invalid control flow or memory access.

The header was also cleaned to remove any semantic indicators (such as dpt_crash) that could expose intent or create a hookable anchor

You can check

This is my last attempt.

@luoyesiqiu
Copy link
Copy Markdown
Owner

luoyesiqiu commented Dec 14, 2025

@Cynthia-cnn
Thank you for your patient revisions once again. I apologize if I did not clearly convey the details regarding the dpt_crash function earlier. Please allow me to re-explain it.

The dpt_crash function currently in the project operates on a basic principle: it clears the Link Register (LR). When an attacker debugs the program, if dpt-shell detects the debugging activity, it will actively call the dpt_crash function, triggering a program crash. This forces the attacker to terminate the debugging process; moreover, they cannot view the correct crash stack in Logcat’s crash logs, making it difficult to identify the specific function (or its address) that caused the crash. Even if they attempt to bypass the protection, locating the crash-triggering position becomes relatively more troublesome.

Your approach of obfuscating the function name is actually unnecessary. During the build process, the compiler automatically strips function names—and this is precisely why I did not obfuscate the function name in the first place.

That said, your submitted code introduces many excellent debugger detection methods and fixes long-standing bugs. I genuinely hope dpt-shell can incorporate these detection capabilities and bug fixes. However, you mentioned this would be your final submission, so I am unsure if further modifications will be possible. If we are fortunate enough to receive additional updates from you, I hope the existing functionality of the dpt_crash function can be preserved.

Thank you again for your time and efforts.

@Cynthia-cnn
Copy link
Copy Markdown
Author

@luoyesiqiu

Thank you very much for the detailed explanation and clarification.

I now understand the original design intent of dpt_crash().

My changes were made based on a different model, and since I won't be pursuing further revisions to this project, I think it's best not to force a design direction that deviates from the original dpt-shell philosophy.

I appreciate your patience in reviewing my submission, and I'm glad that some of the improved detection logic and bug fixes are useful.
Thank you again for maintaining this project and for your thoughtful feedback.

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.

2 participants