superandroidanalyzer / super Goto Github PK

Tests are failing in 32 bit Windows, even if they work in 64 bit. The error is assertion failed: config.check(), which shouldn't happen. Here the build. Interestingly enough, the build with Rust 1.11 in GNU works in i686.

We need to create a complete rule specification, and publish it as an RFC. We have to include current rule specification and decide if we require more fields, or if we want to modify current ones.

It would be nice to have some guidelines on how to use this on the README. I'm thinking of simple steps like "install, config, compile, download apk, run".

At this moment, URLs that start by "content://" are detected by this rule. This should not happen since we do not want to include the Content Provider URLs as URL disclosure.

It is not so easy as using a Whitelist since the matching part is not the "content://" but the URL that comes after it.

We currently use Strings in the Config struct to store paths. This should be avoided, since we have PathBuf structs for that purpose, that they even have great methods for path composition, that will give us even better multiplatform support.

We need to stablish an RFC process similar to the one in Rust, so that new features can be requested by anyone, commented and published, and finally implemented. We need to specify a protocol for those requests.

I would say we should take an approach similar to the one Rust is taking.

Currently, references to R file veriable IDs are pure integers. We should reverse them using tools like this one: https://github.com/justingarrick/android-reverse-r

Currently we use jd-cmd as a dependency for our project, which is a java decompiler. We should use our own so that we don't depend on Java.

It would be great to have multilanguage support in SUPER, being able to select the language in configuration files. We could support Spanish and English out of the box, but I think we should have a simple language-file system. I'm still not sure how to implement this with rules, though.

Users should be able to override configuration options through command line, by using CLI options such as --threads for the configuration options. Permissions vulnerability configuration would only be in the config file, and they could be disabled by using --disable-permission=perm1,perm2,perm3, or with another delimiter specified by using the use_delimiter() method in clap::Arg

Debug and large heap lines are not set in the manifest report. We only need to check for the attributes in the file.

This will be implemented with #25, but in any case we need to create tables for vulnerabilities in summary reports, with vulnerabilities grouped by type. Also, the ability to understand that if the same vulnerability (in cases such as superuser detection) occurs in the same function, it will probably be only one vulnerability.

At this moment, all regex execute over all files. It would be nice to separate the regex rules by what are they supposed to do. This way the regex analysis would be much more efficient. It could be something like this:

• Source code regex (.java)
• XML regex
• Manifest regex

This can be done adding a new attribute in the rules, it shouldn't be hard.

We need to package the application for different platforms:

• Windows - #17
• MacOS X - #18
• Linux - #16

We need to add content providers, receivers, etc. to manifest analysis, and rules to decide what to analyze. Here is the complete list:

• <provider>:
  • if android:exported="false" everything is OK.
  • if android:exported="true" and any targetSdkVersion or no android:exported and minSdkVersion < 17, we could have a vulnerability:
    • if android:permission or android:readPermission or android:writePermission, only warning.
    • if no permissions, medium or high vulnerability: other apps can read it.
• <receiver>, <activity>, <activity-alias> or <service>:
  • if android:exported="false" everything is OK.
  • if android:exported="true" we could have a vulnerability:
    • if android:permission, only warning.
    • if no permissions, medium or high vulnerability: other apps can access it.
  • If no android:exported, we could have a vulnerability:
    • if no <intent-filter>, everything is OK.
    • if <intent-filter>:
      • if android:permission, only warning.
      • if no permissions, medium or high vulnerability: other apps can access it.

Analysis from AndroBugs: https://github.com/AndroBugs/AndroBugs_Framework/blob/master/androbugs.py

Right now, the dangerous permissions in config.toml are these ones:
https://developer.android.com/guide/topics/security/permissions.html?hl=es#perm-groups

But there are a lot of potentially dangerous permissions not included. We should look and add a few more. For example:
http://androidpermissions.com/
http://stackoverflow.com/questions/3437532/whats-are-the-ten-most-deadly-permissions
http://www.makeuseof.com/tag/the-seven-deadly-android-permissions-how-to-avoid-the-sin-of-slothful-preparedness/

We should create a downloader that downloads the application from the market given only the application package id.

Using Java dependencies is causing decompilation errors. This should be fixed when we implement #22. Some of the errors are shown here:

With com.google.android.apps.photos:

Error: The decompression command returned an error. More info: Exception in thread "main" java.lang.NullPointerException
    at brut.androlib.res.data.value.ResStyleValue.serializeToResValuesXml(ResStyleValue.java:58)
    at brut.androlib.res.AndrolibResources.generateValuesFile(AndrolibResources.java:516)
    at brut.androlib.res.AndrolibResources.decode(AndrolibResources.java:267)
    at brut.androlib.Androlib.decodeResourcesFull(Androlib.java:131)
    at brut.androlib.ApkDecoder.decode(ApkDecoder.java:108)
    at brut.apktool.Main.cmdDecode(Main.java:163)
    at brut.apktool.Main.main(Main.java:81)

With com.alibaba.aliexpresshd:

Error: The decompression command returned an error. More info: Exception in thread "main" brut.androlib.AndrolibException: Could not decode arsc file
    at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:52)
    at brut.androlib.res.AndrolibResources.getResPackagesFromApk(AndrolibResources.java:558)
    at brut.androlib.res.AndrolibResources.loadMainPkg(AndrolibResources.java:72)
    at brut.androlib.res.AndrolibResources.getResTable(AndrolibResources.java:64)
    at brut.androlib.Androlib.getResTable(Androlib.java:67)
    at brut.androlib.ApkDecoder.setTargetSdkVersion(ApkDecoder.java:193)
    at brut.androlib.ApkDecoder.decode(ApkDecoder.java:102)
    at brut.apktool.Main.cmdDecode(Main.java:163)
    at brut.apktool.Main.main(Main.java:81)
Caused by: java.io.IOException: Expected: 0x001c0001, got: 0x00000001
    at brut.util.ExtDataInput.skipCheckChunkTypeInt(ExtDataInput.java:75)
    at brut.util.ExtDataInput.skipCheckChunkTypeInt(ExtDataInput.java:73)
    at brut.androlib.res.decoder.StringBlock.read(StringBlock.java:44)
    at brut.androlib.res.decoder.ARSCDecoder.readTableHeader(ARSCDecoder.java:75)
    at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:47)
    ... 8 more

And a possibly unrelated issue in com.google.android.apps.fireball:

Error: There was an error when decompressing the .apk file. More info: Io Error: Could not read enough bytes

We should not require Java dependencies for our software, here is the current status of Java dependencies:

• APKTool - #20
• jd-cmd - #23
• dex2jar - #24

We need packages for MacOS X.

Hi,
I think there's a typo in the Github description.

Analizer should be Analyzer.

The project is now public, and to allow newcomers we should create a Contributing.md file explaining how to contribute to the project.

As a result of every application analisys process, a results.json file is created which contains detected vulnerabilities and their criticality level. We could use this information for statistics to be shown on the Website. What do you think?

We should be able to extract classes from .dex files without requiring dex2jar, and that way depending on less Java code.

We should modify the web page to show our first release, and to show download links.

Currently OpenSSL dependency is not added to .deb files, which prevents certificate analysis.

We are using an script to analyze all the apks located in /downloads. We should consider to add this functionality as an option flag during program execution. Also a brief explanation of its usage would be requiered. What do you think?

It would be great to be able to highlight a given line (or even lines) passing URL parameters to HTML files with source code.

As we are not sure if the decompiler process is reliable as some information is missing (Try and catch structures, for loops, while loops) we should remove those rules until we find how the decompiler process works.

Having a command line option such as --open (the same way as in cargo doc --open) to open the HTML report once generated would be really useful, and I think it wouldn't take much.

It would be as simple as using the open crate to open the generated index.html file.

Currently certificate analysis is only done by an OpenSSL dependency and the certificate is not correctly analyzed. We should probably do our own native analysis and get some output in the report.

The idea behind this is replacing the OpenSSL dependency, can it be done with ring? or with our own PKSC#7 parser?

We are currently getting too many false positives with the password rule. We must find a way to reduce that.

We need to improve results template so that it doesn't look, well, like it does.

We are currently using the yaml-rust crate for YAML parsing, but it does not use the benefits of serde, and we can benefit from it (which we already use with JSON) for YAML using serde_yaml. Also, in one or two releases, we will have the custom derive traits in Rust, which will enable us to remove a lot of code by using serde with its currently nightly features. Also, using serde we will benefit from macros 1.1 in the future, when they reach Rust stable.

We must package the project in .deb for Debian based distributions such as Ubuntu, Kali or Mint, and .rpm for RedHat based distributions, such as OpenSUSE, Fedora or CentOS.

We must package the application for Windows 8.1+

As it can be seen here build in Unix fail.

The strings in the Manifest, such as @string/app_name are not resolved and that gives pretty ugly outputs in the report.

Currently we show the time of the report but not the SUPER version that it was used to generate it.

We should use Moustache/Handlebars templates for report generation.

Currently, all vulnerabilities appear directly in the report. We shiould provide a way to collapse them so that we can easily traverse all different kind of vulnerabilities.

Here is a list of vulnerabilities we should try to catch:

LOW - MEDIUM:

• URL disclosure (Medium)
  • RegEx: "(((ftp|http)s?:)?//)?[\w\.-]+\.[a-zA-Z]{2,6}(/[\w\./-]*)?"
  • Cases:
    • Should match:
      • "http://www.google.es/"
      • "http://www.razican.com"
      • "https://razican.com"
      • "http://www.razican.com/hello"
      • "//www.razican.com/hello"
      • "ftp://ftp.razican.com/hello"
    • Shouldn't match:
      • "android.intent.extra.EMAIL"
      • "hello"
      • "http://schemas.android.com/apk/res/android"
      • "http://www.w3.org/2005/Atom"
• Generic Exception in catch (Medium or Low)
  Catch an exception and not specifying more in the type of exception (Exception e) (Bad practice).
  • RegEx: catch\s*\(\s*((\s*\|?\s*\w+)*\|)?\s*Exception\s*((\s*\|\s*\w+)*)?\s+\w+\s*\)
  • Cases:
    • Should match:
      • catch (Exception e)
      • catch (IOException|Exception e)
      • catch (IOException|Exception|PepeException e)
      • catch (IOException | Exception | PepeException hello)
    • Shouldn't match:
      • catch (IOException e)
      • catch (IOException|PepeException e)
• Generic Exception in throws (Medium or Low)
  Search code for throws Exception.
  • RegEx: throws\s+(\w*\s*,\s*)*Exception\s*[,{]
  • Cases:
    • Should match:
      • throws Exception {
      • throws Exception, IOException {
      • throws IOException, Exception {
      • throws IOException, Exception, PepeException {
      • throws IOException,Exception,PepeException {
    • Shouldn't match:
      • throws IOException {
      • throws IOException, PepeException {
• Hidden fields (Low)
  • RegEx: (setVisible\s*\(\s*View\s*\.\s*(INVISIBLE|invisible)\s*\))|(android\:visibility\s*=\s*"invisible")|(android\:background\s*=\s*"(?i)(\@?null)")
  • Cases:
    • Should match:
      • setVisible(View.INVISIBLE)
      • setVisible(View.invisible)
      • android:visibility = "invisible"
      • android:background = "NULL"
      • android:background = "null"
      • android:background = "@null"
• Unused method or unused variable (Low)
  Search for unused methods and variables through the code. (TODO)
  • RegEx:
  • Cases:
• Empty catch (Low)
  Using catch structure without any performed operations inside.
  • RegEx: catch\s*\((\w|\s|\|)+\)\s*{\s*}
  • Cases :
    • Should match:
      catch ( OutOfMemoryError e1 ) { }
    • Should not match:
      catch(OutOfMemoryError e1) { e1.printStackTrace(); }

HIGH - CRITICAL:

• Null dereference (High)

  In the case that a variable gets initialized to do null or could be null in some assignment, we have to check that the exception gets catched or the variable gets checked. (TODO)

  • RegEx:
  • Cases:

• Harcoded Password (High)
  Clear text passwords in the code.

  • RegEx: ((p((a|@)(s|\$)(s|\$)?(w(o|0)rd(s|\$)?)?|wr?d?|(a|@)(s|\$)(s|\$)?wd)|(p(((s|\$)(s|\$)?(w(o|0)?r?d(s|\$)?)?)|rd))))
  • Whitelist : p$$ ps pw prd pwr pss p$
  • Patterns: password, passwords, pass, pswd, pwd, psswd, psswrd, pssword, psword, pasword, pwrd, pswd, p@ssword, p@sswords, p@ss, p@sword , password, passwords, pass, pswd, pwd, psswd, psswrd, pssword, psword, pasword, pwrd, pswd passwd
  • Cases:
    • Should match:
      • password = "secret";
      • passwords = "secret";
      • pass = "secret";
      • pwd = "secret";

• Empty password (High)
  Empry passwords with syntax such as String passwd = “”; (TODO)

  • RegEx: ((p((a|@)(s|\$)(s|\$)?(w(o|0)rd(s|\$)?)?|wr?d?|(a|@)(s|\$)(s|\$)?wd)|(p(((s|\$)(s|\$)?(w(o|0)?r?d(s|\$)?)?)|rd))))\s*=\s*\"\s*\"\s*;

  • Cases:

  • Should match:

    • password = " " ;

    And any combination of "password" as described above.

• Private IP Disclosure (High)

  • RegEx (IPv4): ((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)
  • RegEx (IPv6): (([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))
  • Whitelist: [^:xdigit:]::[^:xdigit:]
  • Cases:
    • Should match:
      • 192.168.1.1
      • 255.255.255.0
      • 0.0.0.0
      • 2001:db8::ff00:42:8329
      • ::ffff:192.0.2.128
      • 3ffe:1900:4545:3:200:f8ff:fe21:67cf
    • Shouldn't match:
      • 264.123.2.14
      • 122.3214.34.34
      • 3ffe:10900:4545:3:200:f8ff:fe21:67cf
      • 3gfe:1900:4545:3:200:f8ff:fe21:67cf
      • 3ffe:4545:3:200:f8ff:fe21:67cf
      • ::

• Allocated memory (High)
  Not freeing reserved resources, i.e. catch an exception and not freeing the memory in the finally() clause. (TODO)

  • RegEx:
  • Cases:

• Insecure random function (High)
  The Math.random() is a photographically insecure random number generator.

  • RegEx: Math\s*\.\s*random\s*\(\s*\)
  • Cases:
    • Should match:
      • Math.random()

• Harcoded file separator (High)
  Paths like C:\Program Files\... can cause problems, and are considered vulnerabilities, since some OSs use backslashes \ (DOS/Windows) and others slashes / (Unix).

  • Windows
    • RegEx: ([a-zA-Z]\:)?\\(\s*)[^\0 !$&*()+]\w.+`
      • Or: (?:[a-zA-Z]\:(\\|\/)|file\:\/\/|\\\\|\.(\/|\\))([^\\\/\:\*\?\<\>\"\|]+(\\|\/){0,1})+
    • Cases:
      • Should match: C:\Program Files\Microsoft\Age.exe
  • Unix
  • RegEx: =(\s*)\"(\s*)\/(\s*)[^\0/ !$&()+].+(\s)"`
  • Cases:
    • Should match: = "/etc/temp/library/lib.txt "

• Non checked input parameters in logs (High)
  Using variables that receive data introduced by the user in logs can be a vulnerability, since they could add private information to the logs..

  • RegEx: Log(\s)*\.(\s)*(w(tf)?|e|d|i|v)+(\s)*\(((\s)*\"?([A-Za-z0-9])*(.)*\"?(\s)*)\,((\s)*\"([A-Za-z0-9])*(.)*\"(\s)*\+)?(\s)*([A-Za-z0-9.\(\)\[\]-])*(\_([A-Za-z0-9.\(\)\[\]-])*)*(\s)*((\+(\s)*([A-Za-z0-9.\(\)\[\]-])*(\_([A-Za-z0-9.\(\)\[\]-])*)*(\s)*)*)?\)(\s)*\;
  • BlackList: e.getMessage()
  • Cases:
    • Should match:
      • Log.d("Diva-sqli", "Error occurred while searching in database: " + mensajeAMostrar);
      • Log.d("Diva-sqli", "Error occurred while searching in database: " + MensajeAMostrar + msg1 + msg2 + msg3);
    • Shouldn't match:
      • Log.e("Hello!")
      • Log.e("Hello: " + var)
      • Log.e("Hello: " +var)
      • Log.wtf("Hello: "+var)
      • Log.i(var)
      • Log.println("Hello: " + var + " goodbye")

• Hardcoded Passwords (comparing strings) (High)

  Hardcoded password comparation following the structure "String.equals(password)" and "password.equals("string")

  • RegEx:(")\s_[^\0/ !$`&_()+]\s(")?.\s_equals\s_(\s_((p((a|@)(s|$)(s|$)?(w(o|0)rd(s|$)?)?|wr?d?|(a|@)(s|$)(s|$)?wd)|(p(((s|$)(s|$)?(w(o|0)?r?d(s|$)?)?)|rd))))\s_)
  • Cases :
    • Should match:
      • "myPass".equals(password)
    • RegEx:((p((a|@)(s|$)(s|$)?(w(o|0)rd(s|$)?)?|wr?d?|(a|@)(s|$)(s|$)?wd)|(p(((s|$)(s|$)?(w(o|0)?r?d(s|$)?)?)|rd))))\s_.\s_equals\s_(\s_"\s_[^\0/ !$`&_()+]\s"\s*)
    • Cases :
    • Should match:
      • password.equals("myPass")

• TextView or EditText password exposure (High)
  Password exposured using ".setText()" method.

  • RegEx:.setText[A-Za-z0-9_- "()+/$@=.,;:ºª¨#·%&'¡!¿?`´{}<>çÇ|]+\s*((p((a|@)(s|$)(s|$)?(w(o|0)rd(s|$)?)?|wr?d?|(a|@)(s|$)(s|$)?wd)|(p(((s|$)(s|$)?(w(o|0)?r?d(s|$)?)?)|rd))))
  • Cases :
    • Should match:
      • myAwesomeTextView.setText (" My Awesome password" + psswd );
      • myAwesomeTextView.setText (" My Awesome password" + hola + hola+psswd);
        -myAwesomeTextView.setText ( " My Awesome password " + h ol a + psswd + hola);
    • Should not match:
      • myAwesomeTextView.setText(" Set your awesome password " ) ;
      • myAwesomeTextView.setText(" Set your awesome password " + hola ) ;

• Sleep() method with vars inside (High)

  • RegEx:(Thread|thread)\s_.\s_sleep\s_(\s_((\d+(\s_+\s_\d_\s_)\s+\s_[A-za-z]+((\s_+\s_(\d|[A-za-z])))?)|([A-za-z]+\s_(+\s_(\d|[A-za-z])(\s+\s_(\d|[A-za-z])))?))\s_)\s_;
  • RegEx: SystemClock\s_.\s_sleep\s_(\s_((\d+(\s_+\s_\d_\s_)\s+\s_[A-za-z]+((\s_+\s_(\d|[A-za-z])))?)|([A-za-z]+\s_(+\s_(\d|[A-za-z])(\s+\s_(\d|[A-za-z])))?))\s_)\s_;
  • Cases :
    • Should match:
      • SystemClock.sleep(Usertime):
      • Thread.sleep(Usertime+Variable+Variable);
      • Thread.sleep(Usertime+13+123+1+24);
      • Thread . sleep (200+asdad+adasasda );
      • Thread . sleep (200+asdad+adasasda+30 );
      • Thread.sleep(10 + 10 + 10241 + Usertime);
    • Should not match:
      • Thread.sleep(2000);
      • SystemClock.sleep(1000):
      • Thread.sleep(“1000” + Usertime);
      • Thread.sleep();

• SQL Injection (Critical)
  Creating queries with user input data concatenated without validating them or using prepared statements is a vulnerability. We can check queries where strings get concatenated. (TODO)

  • RegEx:
  • Cases :

• XSS (Critical)
  We still have to check how to search for it... (TODO)

  • RegEx:
  • Cases:

• Leaking Content Provider (Critical)
  Initially <provider>s can be identified and warn about the potential vulnerability that should be checked. In a dynamic analysis that could be automatized. (TODO)

  • RegEx:
  • Cases:

• Storing sensitive information (such as passwords) in memory without cleaning it after using it is a vulnerability. (High)

  • Ej: char password[] = pass.getPasswd();
  • Array.fill(password, “”); -> This should be performed when the array is no longer used.
    We could check that anytime a password is loaded in meory a try-catch-finally is performed with memory being overwritten in the finally clause, or just after using the variable, but we still have to check how to do it... :-S (TODO)
  • RegEx:
  • Cases:

• Weak Algorithms (High o critical)
  Using weak algorithms such as “DES”, “MD5”…

  • RegEx: (DESKeySpec)|(getInstance\((\s)*\"(\s)*(md5|MD5)(\s)*\"(\s)*\))|(getInstance\((\s)*\"(\s)*(sha-1|SHA-1)(\s)*\"(\s)*\))
  • Cases:
    • Should match:
      • DESKeySpec
      • getInstance(MD5)
      • getInstance("MD5")
      • getInstance(SHA_1)
      • getInstance("SHA-1")

OTHER IMPLEMENTATIONS (OWASP Methodology):

• Invalid SSL Certificate (Medium)
  Application developer's certificate validity must be checked (Could we check the certificate chain?)
• Autocomplete is not set to OFF
• Application build contains Obsolete Files
• Failure to implement trusted Issuers
• Activity Hijacking
• Service Hijacking
• Broadcast Thief
• Malicious Broadcast Injection
• Malicious Activity/Service Launch
• Insecure Permissions on Unix domain sockets
• Insecure use of network sockets

Seems that if the it_config test is executed before the it_config_sample test, the test fails. We'll have to check that and fix it before next release.

We should not require the use of APKTool to decompile the software.

Currently there is no tests for the manifest. We should create a sample manifest with some of the known vulnerabilities, and check if the vulnerabilities are being detected. Even if we know they work, those tests are important for continuous integration.

It would be great to have a search option in the source viewer, so that we can search strings etc. manually.

The logo should be SVG so that we can used in the required resolution.

We don't do CI for packaging, which we should be doing in order to make sure that packages for different distributions work as expected.

We currently have some coloring errors in the console, this should be fixed for the next version (or hopefully, if it's really easy to fix, for 0.1.0.

An example:

We should test the validity of that code, and add rules back if they really are working correctly.