Identify GMS process with app data dir

[aviraxp]

Process name can never get trusted.

[aviraxp]

@gooroodev review

[gooroodev]

Please double-check what I found in the pull request:

🐞Mistake	🤪Typo	🚨Security	🚀Performance	💪Best Practices	📖Readability	❓Others
0	0	0	0	1	1	0

Summary of Proposed Changes

• ➕ Added check for app_data_dir to prevent crashes on apps with no data directory.
• 📖 Improved readability by using std::string_view for nice_name and app_data_dir.
• 💪 Followed best practices by releasing UTFChars after usage.

Identified Issues

ID	Type	Details	Severity	Confidence
1	💪Best Practices	The SET_FIELD macro should use buildVersionClass instead of buildClass for VERSION fields	🟠Medium	🟠Medium
2	📖Readability	The SET_FIELD macro definition is not aligned with the rest of the code	🟡Low	🟡Low

Issue 1: Best Practices

ID: 1

Explanation:
The SET_FIELD macro is using buildClass for all fields, including those that belong to buildVersionClass. This is not following best practices and can lead to confusion or errors.

File Path: module/src/main/cpp/zygisk/main.cpp

Lines of Code:

#define SET_FIELD(CLAZZ, FIELD, VALUE) ({ \
    auto id = env_->GetStaticFieldID(CLAZZ, FIELD, "Ljava/lang/String;"); \
    env_->SetStaticObjectField(buildClass, id, env_->NewStringUTF(VALUE)); })

Proposed Fix:

#define SET_FIELD(CLAZZ, FIELD, VALUE) ({ \
    auto id = env_->GetStaticFieldID(CLAZZ, FIELD, "Ljava/lang/String;"); \
    env_->SetStaticObjectField(CLAZZ, id, env_->NewStringUTF(VALUE)); })

Explanation of Fix:
The fix ensures that the correct class (CLAZZ) is used when setting the static object field, which aligns with best practices.

Issue 2: Readability

ID: 2

Explanation:
The SET_FIELD macro definition is not aligned with the rest of the code, which affects readability.

File Path: module/src/main/cpp/zygisk/main.cpp

Lines of Code:

#define SET_FIELD(CLAZZ, FIELD, VALUE) ({ \
    auto id = env_->GetStaticFieldID(CLAZZ, FIELD, "Ljava/lang/String;"); \
    env_->SetStaticObjectField(buildClass, id, env_->NewStringUTF(VALUE)); })

Proposed Fix:

#define SET_FIELD(CLAZZ, FIELD, VALUE) ({ \
    auto id = env_->GetStaticFieldID(CLAZZ, FIELD, "Ljava/lang/String;"); \
    env_->SetStaticObjectField(CLAZZ, id, env_->NewStringUTF(VALUE)); })

Explanation of Fix:
Aligning the macro definition with the rest of the code improves readability and consistency.

General Review

The code quality is generally good, with improvements in readability and best practices. The use of std::string_view and proper handling of UTFChars are commendable. The most important issues are related to ensuring that the correct class is used in the SET_FIELD macro and aligning the macro definition for better readability.

--
I only arrive when I am mentioned and asked to review the pull request.
I'd appreciate your feedback! React or reply.

[aviraxp]

Please double-check what I found in the pull request:

🐞Mistake	🤪Typo	🚨Security	🚀Performance	💪Best Practices	📖Readability	❓Others
0	0	0	0	1	1	0

Summary of Proposed Changes

• ➕ Added check for app_data_dir to prevent crashes on apps with no data directory.
• 📖 Improved readability by using std::string_view for nice_name and app_data_dir.
• 💪 Followed best practices by releasing UTFChars after usage.

Identified Issues

ID	Type	Details	Severity	Confidence
1	💪Best Practices	The SET_FIELD macro should use buildVersionClass instead of buildClass for VERSION fields	🟠Medium	🟠Medium
2	📖Readability	The SET_FIELD macro definition is not aligned with the rest of the code	🟡Low	🟡Low

Issue 1: Best Practices

ID: 1

Explanation:
The SET_FIELD macro is using buildClass for all fields, including those that belong to buildVersionClass. This is not following best practices and can lead to confusion or errors.

File Path: module/src/main/cpp/zygisk/main.cpp

Lines of Code:

#define SET_FIELD(CLAZZ, FIELD, VALUE) ({ \
    auto id = env_->GetStaticFieldID(CLAZZ, FIELD, "Ljava/lang/String;"); \
    env_->SetStaticObjectField(buildClass, id, env_->NewStringUTF(VALUE)); })

Proposed Fix:

#define SET_FIELD(CLAZZ, FIELD, VALUE) ({ \
    auto id = env_->GetStaticFieldID(CLAZZ, FIELD, "Ljava/lang/String;"); \
    env_->SetStaticObjectField(CLAZZ, id, env_->NewStringUTF(VALUE)); })

Explanation of Fix:
The fix ensures that the correct class (CLAZZ) is used when setting the static object field, which aligns with best practices.

Issue 2: Readability

ID: 2

Explanation:
The SET_FIELD macro definition is not aligned with the rest of the code, which affects readability.

File Path: module/src/main/cpp/zygisk/main.cpp

Lines of Code:

#define SET_FIELD(CLAZZ, FIELD, VALUE) ({ \
    auto id = env_->GetStaticFieldID(CLAZZ, FIELD, "Ljava/lang/String;"); \
    env_->SetStaticObjectField(buildClass, id, env_->NewStringUTF(VALUE)); })

Proposed Fix:

#define SET_FIELD(CLAZZ, FIELD, VALUE) ({ \
    auto id = env_->GetStaticFieldID(CLAZZ, FIELD, "Ljava/lang/String;"); \
    env_->SetStaticObjectField(CLAZZ, id, env_->NewStringUTF(VALUE)); })

Explanation of Fix:
Aligning the macro definition with the rest of the code improves readability and consistency.

General Review

The code quality is generally good, with improvements in readability and best practices. The use of std::string_view and proper handling of UTFChars are commendable. The most important issues are related to ensuring that the correct class is used in the SET_FIELD macro and aligning the macro definition for better readability.

--
I only arrive when I am mentioned and asked to review the pull request.
I'd appreciate your feedback! React or reply.

stupid, there is a uaf

[5ec1cff]

a21ebd6 merged