From fdb437552cc0ecc73cee62324c46278054ad65a1 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 29 Jul 2022 01:10:15 -0400 Subject: [PATCH] clean up android query and tests --- .../CWE/CWE-489/DebuggableAttributeTrue.qhelp | 22 ++++------ .../CWE/CWE-489/DebuggableAttributeTrue.ql | 4 +- .../Security/CWE/CWE-489/DebuggableFalse.xml | 8 ++++ .../Security/CWE/CWE-489/DebuggableTrue.xml | 8 ++++ .../CWE-489/DebuggableAttributeTrue.expected | 2 +- .../query-tests/security/CWE-489/Test.java | 41 ++++++++++++++++++- .../security/CWE-489/TestFalse.xml | 2 +- .../security/CWE-489/TestNotSet.xml | 2 +- .../query-tests/security/CWE-489/TestTrue.xml | 2 +- 9 files changed, 70 insertions(+), 21 deletions(-) create mode 100644 java/ql/src/Security/CWE/CWE-489/DebuggableFalse.xml create mode 100644 java/ql/src/Security/CWE/CWE-489/DebuggableTrue.xml diff --git a/java/ql/src/Security/CWE/CWE-489/DebuggableAttributeTrue.qhelp b/java/ql/src/Security/CWE/CWE-489/DebuggableAttributeTrue.qhelp index 557c61f43aa..e0e8fbf2d98 100644 --- a/java/ql/src/Security/CWE/CWE-489/DebuggableAttributeTrue.qhelp +++ b/java/ql/src/Security/CWE/CWE-489/DebuggableAttributeTrue.qhelp @@ -4,35 +4,31 @@ -

The debuggable attribute in the application section of the AndroidManifest.xml file should -never be enabled in production builds.

- -

ADD MORE/EDIT?

+

When a debugger is enabled it could allow for entry points in the application or reveal sensitive information.

-

Make sure that the debuggable attribute is set to false in production builds.

+

In Android applications either set the android:debuggable attribute to false +or do not include it in the manifest. The default value when not included is false.

-

In the example below, the debuggable attribute is set to true.

+

In the example below, the android:debuggable attribute is set to true.

+ -

The corrected version sets the debuggable attribute to false.

+

The corrected version sets the android:debuggable attribute to false.

+
  • - Java SE Documentation: - Compound Statements. -
  • -
  • - Wikipedia: - Indentation style. + Android Developers: + The android:debuggable attribute.
  • diff --git a/java/ql/src/Security/CWE/CWE-489/DebuggableAttributeTrue.ql b/java/ql/src/Security/CWE/CWE-489/DebuggableAttributeTrue.ql index 58020bb94d9..a7bacaf4250 100644 --- a/java/ql/src/Security/CWE/CWE-489/DebuggableAttributeTrue.ql +++ b/java/ql/src/Security/CWE/CWE-489/DebuggableAttributeTrue.ql @@ -1,5 +1,5 @@ /** - * @name Debuggable attribute enabled + * @name Android debuggable attribute enabled * @description An enabled debugger can allow for entry points in the application or reveal sensitive information. * @kind problem * @problem.severity warning @@ -7,7 +7,7 @@ * @tags security * external/cwe/cwe-489 * @precision very-high - * @security-severity 0.1 + * @security-severity */ import java diff --git a/java/ql/src/Security/CWE/CWE-489/DebuggableFalse.xml b/java/ql/src/Security/CWE/CWE-489/DebuggableFalse.xml new file mode 100644 index 00000000000..7de19373bf3 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-489/DebuggableFalse.xml @@ -0,0 +1,8 @@ + + + + + + + diff --git a/java/ql/src/Security/CWE/CWE-489/DebuggableTrue.xml b/java/ql/src/Security/CWE/CWE-489/DebuggableTrue.xml new file mode 100644 index 00000000000..3700d5356ba --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-489/DebuggableTrue.xml @@ -0,0 +1,8 @@ + + + + + + + diff --git a/java/ql/test/query-tests/security/CWE-489/DebuggableAttributeTrue.expected b/java/ql/test/query-tests/security/CWE-489/DebuggableAttributeTrue.expected index 5db8848b4cb..16c82e46225 100644 --- a/java/ql/test/query-tests/security/CWE-489/DebuggableAttributeTrue.expected +++ b/java/ql/test/query-tests/security/CWE-489/DebuggableAttributeTrue.expected @@ -1 +1 @@ -| TestTrue.xml:7:5:17:30 | debuggable=true | Warning: debuggable attribute enabled | +| TestTrue.xml:7:5:17:30 | debuggable=true | The 'debuggable' attribute is enabled. | diff --git a/java/ql/test/query-tests/security/CWE-489/Test.java b/java/ql/test/query-tests/security/CWE-489/Test.java index 273aafc3566..2e81c169877 100644 --- a/java/ql/test/query-tests/security/CWE-489/Test.java +++ b/java/ql/test/query-tests/security/CWE-489/Test.java @@ -1,2 +1,39 @@ -// No need for Java code since only testing XML files -public class Test { } +package com.example.myapp; + +import android.app.Fragment; +import android.os.Bundle; +import android.view.LayoutInflater; +import android.view.View; +import android.view.ViewGroup; +import android.widget.Button; +import androidx.fragment.app.FragmentActivity; +import androidx.fragment.app.FragmentTransaction; + +public class Test extends FragmentActivity { + + @Override + public void onCreate(Bundle savedInstance) { + try { + super.onCreate(savedInstance); + final String fname = getIntent().getStringExtra("fname"); + FragmentTransaction ft = getSupportFragmentManager().beginTransaction(); + Class fClass = (Class) Class.forName(fname); + ft.add(fClass.newInstance(), ""); // hasTaintFlow + ft.add(0, Fragment.instantiate(this, fname), null); // hasTaintFlow + ft.add(0, Fragment.instantiate(this, fname, null)); // hasTaintFlow + ft.add(0, fClass, null, ""); // hasTaintFlow + ft.add(0, fClass.newInstance(), ""); // hasTaintFlow + ft.attach(fClass.newInstance()); // hasTaintFlow + ft.replace(0, fClass, null); // hasTaintFlow + ft.replace(0, fClass.newInstance()); // hasTaintFlow + ft.replace(0, fClass, null, ""); // hasTaintFlow + ft.replace(0, fClass.newInstance(), ""); // hasTaintFlow + + ft.add(Fragment.class.newInstance(), ""); // Safe + ft.attach(Fragment.class.newInstance()); // Safe + ft.replace(0, Fragment.class.newInstance(), ""); // Safe + } catch (Exception e) { + } + } + +} diff --git a/java/ql/test/query-tests/security/CWE-489/TestFalse.xml b/java/ql/test/query-tests/security/CWE-489/TestFalse.xml index a66f706a3e7..7ab073147b0 100644 --- a/java/ql/test/query-tests/security/CWE-489/TestFalse.xml +++ b/java/ql/test/query-tests/security/CWE-489/TestFalse.xml @@ -3,7 +3,7 @@ xmlns:tools="http://schemas.android.com/tools" package="com.example.happybirthday"> - + - + - +