MAM Factory looping and crashing

[jonathangerbaud]

Android Studio manages to build and launch the app but it crashes immediately with the following error:

AndroidRuntime:     at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)
        at io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater.setFactory2(-ViewPumpLayoutInflater.kt:94)
        at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)
        at io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater.setFactory2(-ViewPumpLayoutInflater.kt:94)
        at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)
        at io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater.setFactory2(-ViewPumpLayoutInflater.kt:94)
        at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)
        at io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater.setFactory2(-ViewPumpLayoutInflater.kt:94)
        at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)
        at io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater.setFactory2(-ViewPumpLayoutInflater.kt:94)
        at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)
        at io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater.setFactory2(-ViewPumpLayoutInflater.kt:94)
        at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)
        at io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater.setFactory2(-ViewPumpLayoutInflater.kt:94)
        at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)
        at io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater.setFactory2(-ViewPumpLayoutInflater.kt:94)
        at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)
        at io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater.setFactory2(-ViewPumpLayoutInflater.kt:94)
        at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)
        at io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater.setFactory2(-ViewPumpLayoutInflater.kt:94)
        at com.microsoft.intune.mam.client.view.OfflineLayoutInflaterManagementBehavior.setFactory2(OfflineLayoutInflaterManagementBehavior.java:22)
        at com.microsoft.intune.mam.client.view.MAMLayoutInflaterManagement.setFactory2(MAMLayoutInflaterManagement.java:42)

This error is displayed so many times that the logcat is filled with it and I can't read the initial error and stacktrace.

Tested on Emulator (API 29) and Galaxy S8 (Android 9) and got the same result.

Intune App SDK for Android (please complete the following information):

• What version of the Intune SDK are you using? Latest
• What platform is your app based in Native Jave + Kotlin

I haven't done anything but adding the SDK. I let the plugin do its work, I haven't modified any class to integrate with SDK.

[bannus]

Hi @jonathangerbaud, thanks for the report. I can reproduce this and I've filed #12814168 to track our investigation internally.

Can you confirm which exact version of the SDK you're using? For diagnostic purposes only, can you see if you able to work around this by adding io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater to excludeClasses in the intunemam build plugin configuration?

[jonathangerbaud]

Hi @bannus,

I'm using version 8.1.1.

Adding io.github.inflationx.viewpump.internal.-ViewPumpLayoutInflater to excludeClasses seems to do the trick.

The app is not crashing anymore and I can use it as usual.

Thanks for this workaround ;)

[meghandaly]

@jonathangerbaud We are happy to hear the workaround has unblocked you 😄

We are looking looking into making a fix in the SDK so that the workaround is no longer necessary, but do not have a timeline for that at this time.

[meghandaly]

Hi @jonathangerbaud thank you for your patience as we investigated this internally.

Unfortunately, the manner in which the view-pump library overrides LayoutInflater conflicts with the MAM SDK's ability to hook the SDK surface. While the workaround we provided does prevent an app crash, it does present a data leak for your application. Ultimately, the MAM SDK and the view-pump library are not compatible.

If possible, we recommend reviewing your usage of the view-pump library and determine whether the supported extensibility options for LayoutInflator (i.e. the various Factories and Filters) can provide what your app requires.

[paul-turner]

Hi @meghandaly, could you expand more on the data leak that the workaround poses? What data is leaked and in what situations is it accessible? With the workaround, what functionality is lost by the SDK?

[ZacSweers]

Hello! One of the original ViewPump authors here, I work with Paul and their team pointed me at this. I'd be super curious what the alleged data leak is too, but in the meantime I dug into this more and found that MAM is actually at fault here and generating invalid bytecode. It's independent of ViewPump and can be reproduced with the following snippet.

class InflaterTest(context: Context) : LayoutInflater(context) {
  override fun cloneInContext(newContext: Context?): LayoutInflater {
    return this
  }

  override fun setFactory2(factory: Factory2?) {
    super.setFactory2(factory)
  }
}

This is a fairly trivial demo, but the relevant part is the super call in setFactory2(). The post-processed bytecode (in dalvik) looks like this

.method public setFactory2(Landroid/view/LayoutInflater$Factory2;)V
    .registers 2
    .param p1, "factory"    # Landroid/view/LayoutInflater$Factory2;
    .annotation system Ldalvik/annotation/MethodParameters;
        accessFlags = {
            0x0
        }
        names = {
            "factory"
        }
    .end annotation

    .line 12
    invoke-super {p0, p1}, Lcom/microsoft/intune/mam/client/view/MAMLayoutInflaterManagement;->setFactory2(Landroid/view/LayoutInflater$Factory2;)V

    .line 13
    return-void
.end method

Where the relevant bit is .line 12

    invoke-super {p0, p1}, Lcom/microsoft/intune/mam/client/view/MAMLayoutInflaterManagement;->setFactory2(Landroid/view/LayoutInflater$Factory2;)V

This is invalid bytecode in two different ways

1. The invoke-super instruction is now invoking the static MAMLayoutInflaterManagement.setFactory2(...) method, which results in a VerifyError at runtime. This is what we see when testing with the latest release (8.6.1) as well.
2. MAMLayoutInflaterManagement.setFactory2(...) method accepts two parameters (original and factory), but MAM has written this only passing a single factory param. This would result in a NoSuchMethodError at runtime even if the first issue was fixed.

There's no way to intercept super calls like this without replacing the superclass itself. I know MAM does that for other classes, but it doesn't appear to do this with LayoutInflater subclasses. The relevant ViewPump code in the OP stacktrace is such a super.setFactory2(...) call here, so it gets broken in the same way.

To bring this back to the original stackoverflow in the OP, I suspect the SDK changed between when OP tested this and when I looked. Looking at the stacktrace, I'm guessing that the previous implementation rewrote the call to look something like this (hand-wavy pseudocode)

-super.setFactory2(factory)
+MAMLayoutInflaterManagement.setFactory2(this, factory)

@paul-turner then built against an earlier version of the SDK (8.1.1) and confirmed the final transformed dalvik code does indeed do that

.method public setFactory2(Landroid/view/LayoutInflater$Factory2;)V
    .registers 2
    .param p1, "factory"    # Landroid/view/LayoutInflater$Factory2;
    .annotation system Ldalvik/annotation/MethodParameters;
        accessFlags = {
            0x0
        }
        names = {
            "factory"
        }
    .end annotation

    .line 14
    invoke-static {p0, p1}, Lcom/microsoft/intune/mam/client/view/MAMLayoutInflaterManagement;->setFactory2(Landroid/view/LayoutInflater;Landroid/view/LayoutInflater$Factory2;)V

    .line 15
    return-void
.end method

This is both functionally different than what was there before (no super call now) and would definitely result in the stackoverflow seen above since MAMLayoutInflaterManagement (as best I can tell) just forwards calls to a cached OfflineLayoutInflaterManagementBehavior that just no-ops and forwards the call to the passed in LayoutInflater param, which is just this again. Interestingly, both the older and newer MAM versions generate the same HTML report for that despite producing different transformations.

I tried to best-effort diagram it here, hope that makes sense

TL;DR: MAM is generating invalid bytecode. Irregardless of its stance on ViewPump as a library, this is a bug that should be fixed, either by not transforming super calls like this or inserting its own MAM LayoutInflater superclass like it does with some other classes. Any LayoutInflater subclass is susceptible to this issue if they override + call super.

[ZacSweers]

Someone on Twitter pointed out an even simpler solution than replacing the superclass: https://twitter.com/JakeWharton/status/1582117262474981376?s=20&t=jQmPuzePRNRAECHm7lz5Qg

You can also generate a synthetic sibling method in the target type which directly calls the superclass method. Then, when you're intercepting the original, your function can now call the new synthetic method to go directly to the super. Easier than synthesizing a whole subclass.

I'm fairly disappointed that this remains unacknowledged two months later. It's an obvious bug with MAM with a detailed explanation of where the issue is, and the closed-source nature of this SDK means that folks can't fix it ourselves. It would be helpful if the maintainers here could at least acknowledge the bug.

Separately, I'd like to explicitly ask the maintainers (@meghandaly @microsoftjoe @msft-cofitz) to reconsider their above claim that ViewPump constitutes a data leak, as it was presented without evidence and seemingly in service of closing a real bug. Classifying good OSS libraries like this shouldn't be done lightly or flippantly, as there's real people on the other side of them and many more that expect MDM services like this to make accurate and informed claims of this nature.

[joakley-msft]

Hi Zac, thanks for raising this again, and apologies for the delay in getting back to you. First of all, I think there's been a slight misunderstanding: it wasn't anyone's intention to imply that View Pump itself created data leaks or other security concerns inherently. The concern was that when combining ViewPump and Intune MAM in an app, suppressing Intune MAM processing on these classes might lead to data leaks due to Intune's view inflation interception not happening.

I do agree with you that Intune should not be generating invalid bytecode, and as you and others have pointed out, that part by itself is not particularly hard to fix. What gets slightly trickier is that because Intune and ViewPump both intercept layout inflation, we have to make sure that in addressing this we get the ordering/interleaving right such that Intune's replacement of certain base Android views still takes effect. Early investigations hit some complexities.

I'm reopening this issue while we investigate further.

[ZacSweers]

Thank you

[B3nedikt]

Hi @joakley-msft & @meghandaly. I got to this issue from an interesting blog post by @ZacSweers. I created a fork of ViewPump which does not rely on a custom LayoutInflater on android >Q. Of course switching to a fork takes a bit of work for your users, but I think its better then just deactivating either your "MAM" product or ViewPump ;)

[bannus]

This was fixed in the 9.4.0 release of the Intune App SDK.