NativeScript / NativeScript/android

Recursion in Util.java. I suspect that this will lead to issues passing a Google Tier2 CASA

未关闭
#1,785 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

主要语言
C++
星标
563
派生
144
平均合并
10 小时 46 分钟
30 天内合并 PR
14

描述

Environment
Provide version numbers for the following components (information can be retrieved by running tns info in your project folder or by inspecting the package.json of the project):

✔ Getting NativeScript components versions information...
✔ Component nativescript has 8.5.3 version and is up to date.
✔ Component @nativescript/core has 8.5.9 version and is up to date.
✔ Component @nativescript/ios has 8.5.2 version and is up to date.
✔ Component @nativescript/android has 8.5.2 version and is up to date.

Describe the bug
Hello together, I am currently preparing for a Tier2 CASA from Google. As I do have to provide Source Code scans using FluidAttacks and it detected a vulnerability in this context of printStackTrace I found the following issue.

https://github.com/NativeScript/android/blob/57388331bf85473d6c3cf5f115dd9d290b80d59b/test-app/app/src/main/java/com/tns/Util.java#L28

public static boolean isDebuggableApp(Context context) {
        int flags;
        try {
            flags = context.getPackageManager().getPackageInfo(context.getPackageName(), 0).applicationInfo.flags;
        } catch (NameNotFoundException e) {
            flags = 0;
            if (Util.isDebuggableApp(context)) {
                e.printStackTrace();
            }
        }

        boolean isDebuggableApp = ((flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0);
        return isDebuggableApp;
    }

It is clear that printStackTrace() is only called in debug mode. However, I consider telling the guys at Google that this is save code as no good idea as it results in a recursion in the catch block. I would suspect that the e.printStackTrace(); in the catch block is actually never called, hence would it not make sense to completly remove it? I have also seen that the runtime provides
a check for isDebuggableApp. Maybe a save alternative would be to just call com.tns.Runtime.isDebuggable() ?

https://github.com/NativeScript/android/blob/57388331bf85473d6c3cf5f115dd9d290b80d59b/test-app/runtime/src/main/java/com/tns/Runtime.java#L248

I do not know the inner workings good enough to know if the runtime always exists at that point. Please consider it as just an idea.

Expected behavior
Catch should not run into a rekursion

Additional context

https://gitlab.com/fluidattacks/universe/-/issues/10406

Currently I am seeing lots of issues in FluidAttacks indirectly referencing this code. Below output is just an example. To my understanding this is a false positive as printStackTrace is not called for production builds. However, also in production builds the recursion would exist to my understanding.

234. Technical information leak - Stacktrace,CWE-209,The error stacktrace can be printed in OWASP/app/src/debug/java/com/tns/ErrorReport.java,CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:N/E:U/RL:U/RC:R,https://docs.fluidattacks.com/criteria/vulnerabilities/234,skims,SAST,92,"
   82 |         final int version = Build.VERSION.SDK_INT;
   83 |         if (version >= 23) {
   84 |             try {
   85 |                 // Necessary to work around compile errors with compileSdk 22 and lower
   86 |                 Method checkSelfPermissionMethod;
   87 |                 try {
   88 |                     checkSelfPermissionMethod = ActivityCompat.class.getMethod(""checkSelfPermission"", Context.class, Stri
   89 |                 } catch (NoSuchMethodException e) {
   90 |                     // method wasn't found, so there is no need to handle permissions explicitly
   91 |                     if (Util.isDebuggableApp(activity)) {
>  92 |                         e.printStackTrace();
   93 |                     }
   94 |                     return;
   95 |                 }
   96 |
   97 |                 int permission = (int) checkSelfPermissionMethod.invoke(null, activity, Manifest.permission.WRITE_EXTERNA
   98 |
   99 |                 if (permission != PackageManager.PERMISSION_GRANTED) {
  100 |                     // We don't have permission so prompt the user
  101 |                     Method requestPermissionsMethod = ActivityCompat.class.getMethod(""requestPermissions"", Activity.class
  102 |
      ^ Col 0
",java.java_info_leak_stacktrace

贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

调研方向

先从 test-app/app/src/main/java/com/tns/Util.java 中引用的 isDebuggableApp catch 块开始,然后检查 test-app/runtime/src/main/java/com/tns/Runtime.java 中的 isDebuggable() 入口点。确认 catch 路径不会递归,并验证由此产生的行为符合预期行为且解决了报告中的扫描问题。

由索引模型根据 Issue 内容生成。

评估

技术栈
android, java
领域
mobile-dev, security
Issue 类型
缺陷
难度
2/5
预计耗时
1-3 小时
活跃度
停滞
描述清晰度
基本清楚
新手友好度
35/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。