no compile. error "duplicate symbol '@null'." in attrs.xml [BUG]

achakian0000 commented 2 years ago

Information

Apktool Version (apktool -version) -2.6.1
Operating System (Mac, Linux, Windows) -Windows
APK From? (Playstore, ROM, Other) -ROM android 12

Stacktrace/Logcat'

I: Using Apktool 2.6.1
I: Checking whether sources has changed...
I: Smaling smali folder into classes.dex...
I: Checking whether sources has changed...
I: Smaling smali_classes2 folder into classes2.dex...
I: Checking whether resources has changed...
I: Building resources...
W: C:\Users\mor\Documents\APK Easy Tool\1-Decompiled APKs\MtkSettings\res\values\attrs.xml:55: error: duplicate symbol '@null'.

Steps to Reproduce

Disassembly and compile without changes

Frameworks

https://drive.google.com/file/d/1CrZKjC0L_pvjm6Rp-Zt_804U80bqe1uv/view?usp=sharing

APK

https://drive.google.com/file/d/1XA3UTWS91GeF72Gdfj852ZLQIZc5kWkw/view?usp=sharing (settings app)

Questions to ask before submission

Have you tried apktool d, apktool b without changing anything? yes
Are you using the latest apktool version? yes

iBotPeaches commented 2 years ago

Confirmed. Marking as bug.

oSumAtrIX commented 1 year ago

Reverting commit dc3667dbd056edf9bd7a41ea5f13324cba9fd4f8 fixes the issue. Might be worth looking into that commit.

GushchinD commented 1 year ago

Same issue on apktool 2.7.0 (Linux) with this APK

iBotPeaches commented 1 year ago

<?xml version="1.0" encoding="utf-8"?>
<resources>
    <attr name="actionBarDivider" format="reference" />
    <attr name="actionBarItemBackground" format="reference" />
    <attr name="actionBarPopupTheme" format="reference" />
    <attr name="actionBarSize" format="dimension">
        <enum name="@null" value="0" />
    </attr>

Did a bit of research while running through all issues. Looks like we have a flaw in decoding where our name resolves to null.

iBotPeaches commented 1 year ago

➜  2836 java -jar apktool_2.7.0.jar d MtkSettings.apk -t 2836 -s -f -o 270
I: Using Apktool 2.7.0 on MtkSettings.apk
I: Loading resource table...
I: Decoding AndroidManifest.xml with resources...
I: Loading resource table from file: /home/ibotpeaches/.local/share/apktool/framework/1-2836.apk
I: Regular manifest package...

➜  2836 java -jar apktool_2.8.0.jar d MtkSettings.apk -t 2836 -s -f -o 280
I: Using Apktool 2.8.0 on MtkSettings.apk
I: Loading resource table...
S: Unknown chunk type: 3eb0

Had an hour before work. Things I discovered.

Between 2.7.0 and 2.8.0 we introduced "Unknown chunk type". What is odd about this as disassembly didn't fail in 2.7.0, so why we are skipping a chunk seems a bit scary.

However, compare 2.7.0, 2.8.0 and 2.8.1 and you'll find its only the regressions to the apktool.yml that show up.

Screenshot from 2023-08-08 07-20-40

The sparse chunk parsing broke the missing res spec parser as we don't properly pad fake resources for ones we fail to decode. This actually worked way more in our favor, but it seems the legacy behavior of padding is what worked here.

So my takeaways and current focus:

Double check the missing res spec parser to make it robust between sparse and non-sparse applications
Double check why this application post 2.7.0 has an unknown chunk that wasn't detected nor crashed on 2.7.0 prior.

cpereirarafa commented 1 year ago

Same issue on apktool 2.8.1 with binance apk https://www.binance.com/en/download

iBotPeaches commented 1 year ago

@cpereirarafa - Why do you say that? I don't see any of the similar crash with that application.

iBotPeaches commented 1 year ago

I spent some time with this and have a patch locally, but I'm not positive if its the right choice.

What is happening is the resourceId given we either cannot resolve (or its legit 0). We can't make those to any known value and fallback through some debatable logic to @null.

This works for the first resource, but as you see becomes duplicate symbols.

I've debated a few paths and they all kinda suck.

We implement unknown resource qualifiers. The XML spec is quite strict, so we'd have to prefix like [res]0x{resId} or something like that in order to stay in the range of allowed characters.
We strip and remove unknown resources entirely as if we can't understand them - then I doubt Android could.
Continue research as there must be a decoding issue and those resources must be defined.
Find out if a true null/blank name exists to reference without duplicate symbols.

  <attr name="actionBarDivider" formats="reference" />
  <attr name="actionBarItemBackground" formats="reference" />
  <attr name="actionBarPopupTheme" formats="reference" />
  <attr name="actionBarSize" formats="dimension">
    <enum name="res0x7f090459" value="0" />
  </attr>

This instead of the existing:

      <attr name="actionBarDivider" format="reference" />
    <attr name="actionBarItemBackground" format="reference" />
    <attr name="actionBarPopupTheme" format="reference" />
    <attr name="actionBarSize" format="dimension">
        <enum name="@null" value="0" />
    </attr>

iBotPeaches commented 1 year ago

tldr - I am bringing back dummy resources.

long version.

➜  2836 aapt2 d resources Kate.apk| grep -i '0x7F090459' -b3 -a3
14054-      () (attr) type=reference
14085-    resource 0x7f040003 attr/actionBarSize
14128-      () (attr) type=dimension|enum size=1
14171:        0x7f090459=0x00000000
14201-    resource 0x7f040004 attr/actionBarSplitStyle
14250-      () (attr) type=reference
14281-    resource 0x7f040005 attr/actionBarStyle

Take a sample in this issue. We have a resource above known as actionBarSize with 1 child key/pair. However, the value is pointing to a resource that does not exist (split?).

➜  2836 aapt d resources Kate.apk | grep -i '0x7F090459'

So prior to the sparse fixes for a sparse resource table. We to an extreme point padded each res type full of unknown resources. Once we parsed one - we'd simply remove that one from the pool. So once the parsing was done we'd synthesize and create these dummies at the end.

Once done we had an entire pool of resources of dummy resources mixed in at the end with the real resources. This would come into handy above because instead when we disassemble the name of a resources and can't find it - we'd fallback to an apktool dummy resource.

This was accidentally broken during the move to sparse resources years ago - https://github.com/iBotPeaches/Apktool/commit/dc3667dbd056edf9bd7a41ea5f13324cba9fd4f8, but it was already "kinda" broken at that point due some regressions when aapt2 came along.

We just have to be very careful bringing back dummy resources as they caused a fair share of issues with the more restricted aapt2. When you are making fake/dummy resources you'd think matching the type is the best, but how do you make a fake type of say like dimension/float? You could do 0, etc but it was safer to introduce a reference to a resource that resolved to null.

This was patched with removal of dummies with a simple encoding to "@null" and worked quite well. Where it fell apart is what you see here and other places is we replaced the name with "@null" as well - that is completely invalid.

cpereirarafa commented 1 year ago

@cpereirarafa - Why do you say that? I don't see any of the similar crash with that application.

@iBotPeaches Apart from the issue commented on https://github.com/iBotPeaches/Apktool/issues/3183#issuecomment-1700625582, that I have changed as commented and worked, several of the resources in attrs.xml on the binance application are resolved to @null, I will leave the log below.

I: Building resources...
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:60: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:59: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:70: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:69: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:72: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:69: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:73: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:69: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:78: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:77: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:88: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:87: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:89: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:87: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:90: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:87: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:91: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:87: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:92: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:87: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:117: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:116: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:144: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:143: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:145: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:143: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:146: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:143: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:147: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:143: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:148: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:143: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:152: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:151: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:153: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:151: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:154: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:151: note: first defined here.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:155: error: duplicate symbol '@null'.
W: /Users/rafaelcpereira/Documents/binance/res/values/attrs.xml:151: note: first defined here.

The attrs.xml: attrs.zip

The commands (executing from intellij): d --use-aapt2 -f binance.apk -o binance b --use-aapt2 -o binance.apk binance/

The binance version is 2.69.3

iBotPeaches commented 1 year ago

Thanks @cpereirarafa - that does look similar. I'm experimenting with skipping unknown resources entirely while I work on bringing dummies back if you want to give it a spin - https://github.com/iBotPeaches/Apktool/pull/3309

I'm guessing that PR will fail miserably because we remove resources at write-time and they made be needed somewhere.

iBotPeaches commented 1 year ago

On merge of this: https://github.com/iBotPeaches/Apktool/pull/3318

This will close this issue. That might not get you all the way to a recompilation, but I feel the original reported issue is now resolved between the few samples provided.

As all the samples have slightly different failures after this resolution. We can track those elsewhere.

iBotPeaches / Apktool