New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Large numbers in manifest casted to int, overflow MAX_INT #767

Closed
iBotPeaches opened this Issue Mar 18, 2015 · 13 comments

Comments

Projects
None yet
3 participants
@iBotPeaches
Owner

iBotPeaches commented Mar 18, 2015

Original issue 657 created by lvs.tourist on 2014-07-15T23:32:13.000Z:

What steps will reproduce the problem?

  1. Disassemble a Corona SDK apk file
  2. Rebuild it

What is the expected output? What do you see instead?
A correct APK should be built.

What version of the product are you using? On what operating system?
Apktool v1.5.2, Mac OS X 10.9.2

There are two problems inside AndroidManifest.xml

  1. For some reason package name is wrong and set to "com.ansca.corona".
  2. If Google Play Game Services are used, APP_ID is treated as number, but should be a string. The solution is to prepend the ID with an escaped space character.

<meta-data android:name="com.google.android.gms.games.APP_ID" android:value="\ 555555555555"/>

After that everything is fine.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #1 originally posted by connor.tumbleson on 2014-07-20T14:53:35.000Z:

hmm.

https://developers.google.com/games/services/android/quickstart

Looking here. It says you should use a @string reference

Which I then believe is the same bug as # 526 (broken references in AndroidManifest). Either way, I haven't built my test apk to investigate fully yet. So will leave open.

Owner

iBotPeaches commented Mar 18, 2015

Comment #1 originally posted by connor.tumbleson on 2014-07-20T14:53:35.000Z:

hmm.

https://developers.google.com/games/services/android/quickstart

Looking here. It says you should use a @string reference

Which I then believe is the same bug as # 526 (broken references in AndroidManifest). Either way, I haven't built my test apk to investigate fully yet. So will leave open.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #2 originally posted by connor.tumbleson on 2014-07-20T15:06:22.000Z:

  1. was already fixed in 2.x
  2. Yes duplicated. Strings that "look" like ints are casted to ints, thus exceed MAX_INT and get some weird stuff going on.
Owner

iBotPeaches commented Mar 18, 2015

Comment #2 originally posted by connor.tumbleson on 2014-07-20T15:06:22.000Z:

  1. was already fixed in 2.x
  2. Yes duplicated. Strings that "look" like ints are casted to ints, thus exceed MAX_INT and get some weird stuff going on.
@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #3 originally posted by connor.tumbleson on 2014-07-20T18:47:16.000Z:

We dump decoded attributes into an int[]. This is used all over Apktool, from the instance we iterate through the file to the organized arrays.

This will either be a hacky fix or a tiny overhaul. Either way, thanks for the report. I've isolated the problem.

Owner

iBotPeaches commented Mar 18, 2015

Comment #3 originally posted by connor.tumbleson on 2014-07-20T18:47:16.000Z:

We dump decoded attributes into an int[]. This is used all over Apktool, from the instance we iterate through the file to the organized arrays.

This will either be a hacky fix or a tiny overhaul. Either way, thanks for the report. I've isolated the problem.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Mar 18, 2015

Owner

Comment #4 originally posted by connor.tumbleson on 2015-03-13T14:41:00.000Z:

Issue 780 has been merged into this issue.

Owner

iBotPeaches commented Mar 18, 2015

Comment #4 originally posted by connor.tumbleson on 2015-03-13T14:41:00.000Z:

Issue 780 has been merged into this issue.

@iBotPeaches iBotPeaches added Bug and removed Priority-Medium labels Mar 18, 2015

@iBotPeaches iBotPeaches changed the title from Errors when used to rebuild a Corona SDK's APK file to Large numbers in manifest casted to int, overflow MAX_INT Mar 18, 2015

@simtel12

This comment has been minimized.

Show comment
Hide comment
@simtel12

simtel12 May 1, 2015

Contributor

I'd like to take a crack at this - I think the place to start is brut.apktool/apktool-lib/.../AXmlResourceParser.java:m_attributes; is that correct?

Contributor

simtel12 commented May 1, 2015

I'd like to take a crack at this - I think the place to start is brut.apktool/apktool-lib/.../AXmlResourceParser.java:m_attributes; is that correct?

@simtel12

This comment has been minimized.

Show comment
Hide comment
@simtel12

simtel12 May 1, 2015

Contributor

After a little more investigation:

Meta-data does not support large numbers in Android. It is not possible to read Long values from the metadata bundle - they are stored as integers with all the limitations that integers have. Dumping the values via aapt l -a <apk> shows the same behavior - the value is already truncated.

Example:

Having a metadata in the manifest like this:

<application
        android:icon="@drawable/ic_launcher"
        android:label="@string/app_name"
        android:theme="@android:style/Theme.Holo.NoActionBar.Fullscreen">

        <meta-data android:name="com.google.android.gms.games.APP_ID" android:value="555555555555"/>
...
</application>

Leads to an aapt listing like this:

$ aapt l -a iap-sample-app-release.apk 
.
.
.
      E: meta-data (line=31)
        A: android:name(0x01010003)="com.google.android.gms.games.APP_ID" (Raw: "com.google.android.gms.games.APP_ID")
        A: android:value(0x01010024)=(type 0x10)0x59b108e3

This leads me to believe that all (type 0x10) values have a hard limit of MAX_INT, and the values get truncated when the apk is build initially. I don't believe there is anything that Apktool can do about them.

Another case to consider is if the user modifies the decoded manifest and adds the too-large values there. I haven't yet checked what happens in this case, but it's probably a good idea to at least spit out a warning if not an error. Thoughts?

Contributor

simtel12 commented May 1, 2015

After a little more investigation:

Meta-data does not support large numbers in Android. It is not possible to read Long values from the metadata bundle - they are stored as integers with all the limitations that integers have. Dumping the values via aapt l -a <apk> shows the same behavior - the value is already truncated.

Example:

Having a metadata in the manifest like this:

<application
        android:icon="@drawable/ic_launcher"
        android:label="@string/app_name"
        android:theme="@android:style/Theme.Holo.NoActionBar.Fullscreen">

        <meta-data android:name="com.google.android.gms.games.APP_ID" android:value="555555555555"/>
...
</application>

Leads to an aapt listing like this:

$ aapt l -a iap-sample-app-release.apk 
.
.
.
      E: meta-data (line=31)
        A: android:name(0x01010003)="com.google.android.gms.games.APP_ID" (Raw: "com.google.android.gms.games.APP_ID")
        A: android:value(0x01010024)=(type 0x10)0x59b108e3

This leads me to believe that all (type 0x10) values have a hard limit of MAX_INT, and the values get truncated when the apk is build initially. I don't believe there is anything that Apktool can do about them.

Another case to consider is if the user modifies the decoded manifest and adds the too-large values there. I haven't yet checked what happens in this case, but it's probably a good idea to at least spit out a warning if not an error. Thoughts?

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches May 2, 2015

Owner

Very interesting research. I was gonna say, "well lets check the apk and see how it works", but there is no apk.

If I create an INT in <meta-data> that exceeds MAX_INT. AAPT, as shown above, truncates it. I need to do more testing involving running said application and seeing if the value is parsed correctly on device or malformed during compilation from source -> binary.

Owner

iBotPeaches commented May 2, 2015

Very interesting research. I was gonna say, "well lets check the apk and see how it works", but there is no apk.

If I create an INT in <meta-data> that exceeds MAX_INT. AAPT, as shown above, truncates it. I need to do more testing involving running said application and seeing if the value is parsed correctly on device or malformed during compilation from source -> binary.

@simtel12

This comment has been minimized.

Show comment
Hide comment
@simtel12

simtel12 May 2, 2015

Contributor

It is truncated in-application as well. Using the following block, the value is truncated when read. It is not possible to use bundle.getLong() due to a cast exception.

try {
    ApplicationInfo ai = getPackageManager().getApplicationInfo(activity.getPackageName(), PackageManager.GET_META_DATA);
    Bundle bundle = ai.metaData;
    int myApiKey = bundle.getInt("com.google.android.gms.games.APP_ID");
} catch (NameNotFoundException e) {
    Log.e(TAG, "Failed to load meta-data, NameNotFound: " + e.getMessage());
} 
Contributor

simtel12 commented May 2, 2015

It is truncated in-application as well. Using the following block, the value is truncated when read. It is not possible to use bundle.getLong() due to a cast exception.

try {
    ApplicationInfo ai = getPackageManager().getApplicationInfo(activity.getPackageName(), PackageManager.GET_META_DATA);
    Bundle bundle = ai.metaData;
    int myApiKey = bundle.getInt("com.google.android.gms.games.APP_ID");
} catch (NameNotFoundException e) {
    Log.e(TAG, "Failed to load meta-data, NameNotFound: " + e.getMessage());
} 
@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches May 2, 2015

Owner

Thanks a ton for this research @simtel12

It appears this is a limitation in AOSP itself and nothing apktool can aim to fix. I might whip up a test apk just to log it into the system for records, but you've done the research and proof. Thanks!

Owner

iBotPeaches commented May 2, 2015

Thanks a ton for this research @simtel12

It appears this is a limitation in AOSP itself and nothing apktool can aim to fix. I might whip up a test apk just to log it into the system for records, but you've done the research and proof. Thanks!

@MarcMil

This comment has been minimized.

Show comment
Hide comment
@MarcMil

MarcMil Sep 2, 2015

Hi there,
I have an APK here similar to that one specified in the previous post.

AAPT reports

      E: meta-data (line=31)
        A: android:name(0x01010003)="com.google.android.gms.appstate.APP_ID" (Raw: "com.google.android.gms.appstate.APP_ID")
        A: android:value(0x01010024)="479116460380" (Raw: "479116460380")

So the value is really the String "479116460380" and not the number as integer.
APKTool should, instead of outputting

        <meta-data android:name="com.google.android.gms.games.APP_ID" android:value="479116460380" />
        <meta-data android:name="com.google.android.gms.appstate.APP_ID" android:value="479116460380" />

the output should be escaped e.g. like this:

        <meta-data android:name="com.google.android.gms.games.APP_ID" android:value="\u003479116460380" />
        <meta-data android:name="com.google.android.gms.appstate.APP_ID" android:value="\u0034479116460380" />

according to https://developer.android.com/guide/topics/manifest/meta-data-element.html

Afterwards, aapt builds a manifest identical to the manifest of the source apk file.
One example where this bug occurs is com.smgstudio.onemoreline from the playstore.

MarcMil commented Sep 2, 2015

Hi there,
I have an APK here similar to that one specified in the previous post.

AAPT reports

      E: meta-data (line=31)
        A: android:name(0x01010003)="com.google.android.gms.appstate.APP_ID" (Raw: "com.google.android.gms.appstate.APP_ID")
        A: android:value(0x01010024)="479116460380" (Raw: "479116460380")

So the value is really the String "479116460380" and not the number as integer.
APKTool should, instead of outputting

        <meta-data android:name="com.google.android.gms.games.APP_ID" android:value="479116460380" />
        <meta-data android:name="com.google.android.gms.appstate.APP_ID" android:value="479116460380" />

the output should be escaped e.g. like this:

        <meta-data android:name="com.google.android.gms.games.APP_ID" android:value="\u003479116460380" />
        <meta-data android:name="com.google.android.gms.appstate.APP_ID" android:value="\u0034479116460380" />

according to https://developer.android.com/guide/topics/manifest/meta-data-element.html

Afterwards, aapt builds a manifest identical to the manifest of the source apk file.
One example where this bug occurs is com.smgstudio.onemoreline from the playstore.

@MarcMil

This comment has been minimized.

Show comment
Hide comment
@MarcMil

MarcMil Dec 17, 2015

More research: It is sufficient to escape the value by prepending "\ " (slash and space). If you do that, aapt will consider it a string value and write it out as a string value without the slash and space, thus solving the problem.
So what should apktool do?
If it sees a String value (TYPE_STRING, 0x03) AND the value of the string corresponds to an integer number (or float number also btw), it should prepend "\ " without the quotes to the attribute value, so that

       <meta-data android:name="com.google.android.gms.games.APP_ID" android:value="\ 479116460380" />

Otherwise aapt treats it as an integer value and writes it as integer value (TYPE_INT_DEC), which is wrong.
So this is clearly an apktool issue; aapt cannot know that it is not meant to be an integer.

Also, I can provide a link to an apk which has this problem:
https://www.dropbox.com/s/s4c1fpnnagwe5jl/com.smgstudio.onemoreline.apk?dl=1

MarcMil commented Dec 17, 2015

More research: It is sufficient to escape the value by prepending "\ " (slash and space). If you do that, aapt will consider it a string value and write it out as a string value without the slash and space, thus solving the problem.
So what should apktool do?
If it sees a String value (TYPE_STRING, 0x03) AND the value of the string corresponds to an integer number (or float number also btw), it should prepend "\ " without the quotes to the attribute value, so that

       <meta-data android:name="com.google.android.gms.games.APP_ID" android:value="\ 479116460380" />

Otherwise aapt treats it as an integer value and writes it as integer value (TYPE_INT_DEC), which is wrong.
So this is clearly an apktool issue; aapt cannot know that it is not meant to be an integer.

Also, I can provide a link to an apk which has this problem:
https://www.dropbox.com/s/s4c1fpnnagwe5jl/com.smgstudio.onemoreline.apk?dl=1

@iBotPeaches iBotPeaches self-assigned this Dec 18, 2015

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Dec 18, 2015

Owner

Thanks for the large amount of research on this. That looks simple enough to do.

Owner

iBotPeaches commented Dec 18, 2015

Thanks for the large amount of research on this. That looks simple enough to do.

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Dec 18, 2015

Owner
ibotpeaches@tombstone:~/Downloads/Apktool/Bug767$ apktool d com.smgstudio.onemoreline.apk -f
I: Using Apktool 2.0.3-831765-SNAPSHOT on com.smgstudio.onemoreline.apk
I: Loading resource table...
I: Decoding AndroidManifest.xml with resources...
I: Loading resource table from file: /home/ibotpeaches/apktool/framework/1.apk
I: Regular manifest package...
I: Decoding file-resources...
I: Decoding values */* XMLs...
I: Baksmaling classes.dex...
I: Copying assets and libs...
I: Copying unknown files...
I: Copying original files...
ibotpeaches@tombstone:~/Downloads/Apktool/Bug767$ cat com.smgstudio.onemoreline/AndroidManifest.xml | grep 'APP_ID'
        <meta-data android:name="com.google.android.gms.games.APP_ID" android:value="\ 479116460379"/>
        <meta-data android:name="com.google.android.gms.appstate.APP_ID" android:value="\ 479116460379"/>
ibotpeaches@tombstone:~/Downloads/Apktool/Bug767$ 

Works great. Thanks for the research. Doing some profiling to make sure my changes doesn't slow anything down too much (since the change is on the ResString)

Owner

iBotPeaches commented Dec 18, 2015

ibotpeaches@tombstone:~/Downloads/Apktool/Bug767$ apktool d com.smgstudio.onemoreline.apk -f
I: Using Apktool 2.0.3-831765-SNAPSHOT on com.smgstudio.onemoreline.apk
I: Loading resource table...
I: Decoding AndroidManifest.xml with resources...
I: Loading resource table from file: /home/ibotpeaches/apktool/framework/1.apk
I: Regular manifest package...
I: Decoding file-resources...
I: Decoding values */* XMLs...
I: Baksmaling classes.dex...
I: Copying assets and libs...
I: Copying unknown files...
I: Copying original files...
ibotpeaches@tombstone:~/Downloads/Apktool/Bug767$ cat com.smgstudio.onemoreline/AndroidManifest.xml | grep 'APP_ID'
        <meta-data android:name="com.google.android.gms.games.APP_ID" android:value="\ 479116460379"/>
        <meta-data android:name="com.google.android.gms.appstate.APP_ID" android:value="\ 479116460379"/>
ibotpeaches@tombstone:~/Downloads/Apktool/Bug767$ 

Works great. Thanks for the research. Doing some profiling to make sure my changes doesn't slow anything down too much (since the change is on the ResString)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment