From 2d62d339e2b5606259166ded8c43eae6007b1eda Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 11 May 2022 16:28:38 +0200 Subject: [PATCH 1/2] Ignore R8 minified libs from instrumentation --- .../util/ConstantPoolHelpers.kt | 86 ++++++++++++++++++ .../wrap/WrappingInstrumentable.kt | 17 ++++ .../android/gradle/BaseSentryPluginTest.kt | 3 +- .../SentryPluginWithMinifiedLibsTest.kt | 3 +- .../gradle/instrumentation/VisitorTest.kt | 6 ++ .../fileIO/BrazeImageUtils.class | Bin 0 -> 14545 bytes 6 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/util/ConstantPoolHelpers.kt create mode 100644 plugin-build/src/test/resources/testFixtures/instrumentation/fileIO/BrazeImageUtils.class diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/util/ConstantPoolHelpers.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/util/ConstantPoolHelpers.kt new file mode 100644 index 00000000..8e8b2bd7 --- /dev/null +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/util/ConstantPoolHelpers.kt @@ -0,0 +1,86 @@ +package io.sentry.android.gradle.instrumentation.util + +import java.lang.reflect.Field +import org.objectweb.asm.ClassReader +import org.objectweb.asm.ClassVisitor +import org.objectweb.asm.ClassWriter + +/** + * Looks up for the original [ClassWriter] up the visitor chain by looking at the private `cv` field + * of the [ClassVisitor]. + */ +internal fun ClassVisitor.findClassWriter(): ClassWriter? { + var classWriter: ClassVisitor = this + while (!ClassWriter::class.java.isAssignableFrom(classWriter::class.java)) { + val cvField: Field = try { + classWriter::class.java.allFields.find { it.name == "cv" } ?: return null + } catch (e: Throwable) { + return null + } + cvField.isAccessible = true + classWriter = (cvField.get(classWriter) as? ClassVisitor) ?: return null + } + return classWriter as ClassWriter +} + +/** + * Looks up for [ClassReader] of the [ClassWriter] through intermediate SymbolTable field. + */ +internal fun ClassWriter.findClassReader(): ClassReader? { + val clazz: Class = this::class.java + val symbolTableField: Field = try { + clazz.allFields.find { it.name == "symbolTable" } ?: return null + } catch (e: Throwable) { + return null + } + symbolTableField.isAccessible = true + val symbolTable = symbolTableField.get(this) + val classReaderField: Field = try { + symbolTable::class.java.getDeclaredField("sourceClassReader") + } catch (e: Throwable) { + return null + } + classReaderField.isAccessible = true + return (classReaderField.get(symbolTable) as? ClassReader) +} + +/** + * Looks at the constant pool entries and searches for R8 markers + */ +internal fun ClassReader.isMinifiedClass(): Boolean { + val charBuffer = CharArray(maxStringLength) + // R8 marker is usually in the first 3-5 entries, so we limit it at 10 to speed it up + // (constant pool size can be huge otherwise) + val poolSize = minOf(10, itemCount) + for (i in 1 until poolSize) { + try { + val constantPoolEntry = readConst(i, charBuffer) + if (constantPoolEntry is String && "~~R8" in constantPoolEntry) { + // ~~R8 is a marker in the class' constant pool, which r8 itself is looking at when + // parsing a .class file. See here -> https://r8.googlesource.com/r8/+/refs/heads/main/src/main/java/com/android/tools/r8/dex/Marker.java#53 + return true + } + } catch (e: Throwable) { + // we ignore exceptions here, because some constant pool entries are nulls and the + // readConst method throws IllegalArgumentException when trying to read those + } + } + return false +} + +/** + * Gets all fields of the given class and its parents (if any). + * + * Adapted from https://github.com/apache/commons-lang/blob/master/src/main/java/org/apache/commons/lang3/reflect/FieldUtils.java + */ +private val Class<*>.allFields: List + get() { + val allFields = mutableListOf() + var currentClass: Class<*>? = this + while (currentClass != null) { + val declaredFields = currentClass.declaredFields + allFields += declaredFields + currentClass = currentClass.superclass + } + return allFields + } diff --git a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt index 43f3019b..61778f42 100644 --- a/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt +++ b/plugin-build/src/main/kotlin/io/sentry/android/gradle/instrumentation/wrap/WrappingInstrumentable.kt @@ -4,14 +4,19 @@ package io.sentry.android.gradle.instrumentation.wrap import com.android.build.api.instrumentation.ClassContext import com.android.build.api.instrumentation.ClassData +import io.sentry.android.gradle.SentryPlugin import io.sentry.android.gradle.instrumentation.ClassInstrumentable import io.sentry.android.gradle.instrumentation.CommonClassVisitor import io.sentry.android.gradle.instrumentation.MethodContext import io.sentry.android.gradle.instrumentation.MethodInstrumentable import io.sentry.android.gradle.instrumentation.SpanAddingClassVisitorFactory import io.sentry.android.gradle.instrumentation.util.AnalyzingVisitor +import io.sentry.android.gradle.instrumentation.util.findClassReader +import io.sentry.android.gradle.instrumentation.util.findClassWriter +import io.sentry.android.gradle.instrumentation.util.isMinifiedClass import io.sentry.android.gradle.instrumentation.util.isSentryClass import io.sentry.android.gradle.instrumentation.wrap.visitor.WrappingVisitor +import io.sentry.android.gradle.util.info import org.objectweb.asm.ClassVisitor import org.objectweb.asm.MethodVisitor import org.objectweb.asm.tree.MethodNode @@ -26,6 +31,18 @@ class WrappingInstrumentable : ClassInstrumentable { ): ClassVisitor { val simpleClassName = instrumentableContext.currentClassData.className.substringAfterLast('.') + + val classReader = originalVisitor.findClassWriter()?.findClassReader() + val isMinifiedClass = classReader?.isMinifiedClass() ?: false + if (isMinifiedClass) { + // We only check for minified classes for the WrappingInstrumentable, because it is the + // only one which runs over all classes + SentryPlugin.logger.info { + "$simpleClassName skipped from instrumentation because it's a minified class." + } + return originalVisitor + } + return AnalyzingVisitor( apiVersion = apiVersion, nextVisitor = { methods -> diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/BaseSentryPluginTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/BaseSentryPluginTest.kt index 0653ef38..19de60e5 100644 --- a/plugin-build/src/test/kotlin/io/sentry/android/gradle/BaseSentryPluginTest.kt +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/BaseSentryPluginTest.kt @@ -55,12 +55,13 @@ abstract class BaseSentryPluginTest( repositories { google() mavenCentral() + maven { url 'https://appboy.github.io/appboy-android-sdk/sdk' } } } subprojects { pluginManager.withPlugin('com.android.application') { android { - compileSdkVersion 30 + compileSdkVersion 31 defaultConfig { applicationId "com.example" minSdkVersion 21 diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryPluginWithMinifiedLibsTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryPluginWithMinifiedLibsTest.kt index de42accb..b763a2df 100644 --- a/plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryPluginWithMinifiedLibsTest.kt +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryPluginWithMinifiedLibsTest.kt @@ -7,7 +7,7 @@ class SentryPluginWithMinifiedLibsTest : BaseSentryPluginTest(androidGradlePluginVersion = "7.1.2", gradleVersion = "7.4") { @Test - fun `does not break when there is a play-core obfuscated dependency`() { + fun `does not break when there is a minified jar dependency`() { appBuildFile.writeText( // language=Groovy """ @@ -22,6 +22,7 @@ class SentryPluginWithMinifiedLibsTest : implementation 'com.google.android.gms:play-services-vision:20.1.3' implementation 'com.google.android.gms:play-services-mlkit-text-recognition:18.0.0' implementation 'com.adcolony:sdk:4.7.1' + implementation 'com.appboy:android-sdk-ui:19.0.0' } """.trimIndent() ) diff --git a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/VisitorTest.kt b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/VisitorTest.kt index e477a15f..76c8c981 100644 --- a/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/VisitorTest.kt +++ b/plugin-build/src/test/kotlin/io/sentry/android/gradle/instrumentation/VisitorTest.kt @@ -136,6 +136,12 @@ class VisitorTest( ChainedInstrumentable(listOf(WrappingInstrumentable(), RemappingInstrumentable())), null ), + arrayOf( + "fileIO", + "BrazeImageUtils", + ChainedInstrumentable(listOf(WrappingInstrumentable(), RemappingInstrumentable())), + null + ), arrayOf("okhttp/v3", "RealCall", OkHttp(), null), arrayOf("okhttp/v4", "RealCall", OkHttp(), null) ) diff --git a/plugin-build/src/test/resources/testFixtures/instrumentation/fileIO/BrazeImageUtils.class b/plugin-build/src/test/resources/testFixtures/instrumentation/fileIO/BrazeImageUtils.class new file mode 100644 index 0000000000000000000000000000000000000000..f1f25b9871fcfc68d02939655a93d22fc9fb8360 GIT binary patch literal 14545 zcmbta3w)H-v7h;}$+x>Kun7d%BtQrOlMNw&f|vjrk^nI;NWx=L8aJCIS+d!U`)zn= zee{8%wqE-#wzgVnV_Rz}l_Z*5wYEjA_I2CpZPnY__xtIswbthT=X~Ej5)<5eZ}_pZ zbG|uq&Y3gkKQrgsum9`eM~LVmZ4J|{*I(bh@!FDZf1p1Y=_#o$2^=h0RT78}42Ht~ zcqkgF9EkP=(H;wigMK53hkbsdGSC+c^czD1MoINSf7l4FDjDpp9PkfThD6838`rMC zaN|XgjcrWy7nM-h9SVoy$MEEEFlInKq%T~(cJ=zTCD+-BnVcQbp;#ceEffwiEkmcO z?wJ3YV3jd6I2et^tF{W)IN8)z(J`@bZb;@DNZSIf8 z!=Xr3OEB*5@yGo&OxnQVEUcX6BS$`niTmY?hKEN(@(o7Ode*Ym^4N|0osZq<)ZBKb zX3qwl1FAW-Mfh*eQdAzlk)6`2*&Rt?xAaZu&_=mo3X zvkPpCc+H%3cK2NM%?+vCX{EJ-5_g^@o#UPdnfZ{-2VJnvZr9uk@oiCo+wHM>&jod3 zj5+QCe3MzIJ_V)j!h$(@wmh_%pKFQ|&p?%y9a=$=d+|(Ccsj?uWRlJt_fp}p;^O2C z&6!PEbHM`lGOK5aMVG6dCGOJm_r$1f_X^0aOw1N7i`*BWr3}_9PxkhrrJ`WIdzC!# z;(O|8$sG4%H=>#MJ^uF2$3Kyy!2{5yO>J7>&XT5$Ir|>JS>)`H%d^>S?rccs;E@gm zHcP?d_vKWIQ>|kvXsO-T)!0(Iy`ihFwr)p5S4ZRihOVu?&V~*aXDn!GY@PABw|#eO zXJbo4S4%@nTf5JaYi_HpZ)oq@-62YM?QZLAY-sIdx}-N4&s@ILABjZc>cAUJS!LyW z@Yz|rohd)y4+n;T1;NHhhksx&9P9{PgR`ke2L&D=UDoW6^u(f}o~quMf3PnUFsinO z;sAqf{y;n$J676eO5*&vdxAzh6j9^s4fVwPa7@#hb_7GceQ~D6J;4A_#~iN5QWlF2 zbi`vp{{WL`rV%m3tWacdD4vkaZ&uh4idHqInjtkiBoLB71||orWsZZD1)`C7@F?by z8b>un;#GC#BWTDPiiHGZ5^IkH<5jz3Av|#&4h4^FjSfY6V4P*mDGUon0GK9%?MaHD zLt%38#Dun`Ep|4Z)(0a7V6%P@LzXB$X-3m#wUHiW&z+&8!7zrModCa`{!k1CNwmItW3C8ODKz`E!=HN+l6yA}rn+FZ%_%eNK%9vRFAgt9R zQ`dU%buPr(g9FicFwt?@V02JQcYEi z+Zi>r*M|ngcFIw6x|LgEx7F^87*1SL@sUsurdE|y0j-Nnt9j8+!H`b-GZH+c)0G(s_UrVPj0D3v zU6qkwM5peI1fx3jWF&|j?qEiOgE}EzWMJ^Cbvl%hU`(elQ=SQ-ReR79sd3SJ=)F$5gWiWAa$ZrLK7jQ>KZ5+h zQ14Jor727+W*B3V7{>V!eb`AKq?4G{bmL4C*Xbij6nc_j-9>lF`aen^W7>GWT}^17 zE(*G6Bi${#d=GMp^9bs6uY*p>P}yaTjpdC@b5a#li$4y)8pcNYyJ28k83bJPY5I(l z?xW8#ZAi^-O1f8*+=(|t((*la+BB)}8$^!8C&dh2HQD9 zoNbQw_KIUZ2*+$}?dYs+t!rRfnwE)83d^QAHgJSS9dwq)TvR|22gZAtY2|sxDBT$g zMPta!bozH-#$HK=GwXoi9-&7a^d))>9RaB;X6mT&5moG##?k9>`ig^U>8o&6RT+n) zy`??DgZ`l~64p9S+v)PoRt--oc?rK$Qel+pKSE zY8u~&CbFOB=m{q*{bbrF=R7d<0@@-%(JgexU_b*60_>$9CNyfN6 z79AQaO&7$b{E(g({VyQuB+lJ=I_UIYm`I;0c3kvhIru-KmojsK!^#DI>YxVtS!O+k zRF7XcsE&S#5LGJYr*!7LHa7(Y?L(0`(r2B11FRClzJ`|1EzS<)C$y2B{Gm7U7$Y)?rF`7SYC(DAw%5=;2uO$$)p!2j8Tr`auKh0 z@`b#CY4sZgus6w0Q+yngzmPY|uYloQyxAOQi;FHcyP@8m zZc54*wmPYe>u{X|lr!%QQZe;dYMTH5-JUKU&bVUaEGe^3yT3Z)hR)I2*0r^5cWZrz ziyNuJ!8;gmx`J1Vu~2X8Skz8rbyKr@tE6yoE4Mj$6YpeNkOKd@K7Xtuc=b>)5(w(t zj(`;oMtb9YF7Bk8XK@GbM(|Etu8A9@U9jnr38mf2nR=2P2kM8iYzpg4+)uM>|r zz}MqQoAJtgAA^XIa*)I>%`_%r&5isvi8Ux2=OuPL%^*6z)5*uh#Hpuk4Mzh(bzQ9U z%}9(JTzreUwYO%r69Z}|-sPYUMk#434ohX9Oj>nD8JYGVZN&FvHb$Qs<9!bD@%xc^ zrW+-x%<{VrI?2x;N=r{=r#fT)g9k%_4&1jJI-_39HG*->FX-=4ZJqvJv+&yE;*ao0 zoqQ+Xg(w@3nkIMg$ISzKHwri0La$CdC@Z(~Cs6vO-!L>B9Yn4t7oe&>ckwB8d%&N> zr~)qj!NKn6v8q}{bRI_W3b<$+ig(Kec55(xBpT~)7aYLhYdd%lFVt{4J}sy3K1NL# zkh5u?XXS+L%{qV1$qz`gqdO}4tD+lpK8?-v1EZUhZSapX{6zNw&=$5CA0&Mc?%Ly6mF1Ch=M)**9q@i=M*ewb0O<&{}* zoE%kw!Xr)!@}uJVAp?jnqq_LZf(H-t<1jhwljg-h!9+GZB`bCDSNUtw>70{7Jb+g+ zQx-YX$(jDNKINo-{yNhdOUsne&J#G1^F1Ty@Ee)Ax1rqoTMp`F6inW9_qNV_8VXEx zcobU${9P9n@?3$b@54DlY443({G6E~qTtFo5{mRhkJ$N#=Cw=5F(V!v=m^FRhXS}r zS~<n(zu z;gtDQU+W26?Wg>+S^N_J9MN-J43ScM#uczY6 zp%^;28WBihbz#3@1d+sgtV@$UAtM9@wW+u7l#7c;9d{%)+(P0W65k4F0TNu+F2Hw< z2En7uPV&f9X<-^4qdd2R7NJ$s%Ftsr{^Pd=#9zP%Y31^Xhil1z3K7_X7aSrm-|NeZ zkI^+_blnKO<1souLbu$Hc1`o*GY2v{dQ{-MOUV!R@!h|ZEC{>l0&^cy1&Ak_eE%Xf{M=2LuY9G41M_w zeXWVJylxkd(Nn{8Tg3={^CU$;zH=w#3{ww`_Pvu-5AuE0Q0*0wn?&HINWH0NoVNQ~ zmXFd8n6p;7=ZLBidQp1bv^;X#S^CK^txFs6=jkJQ(jNRO^*~l@?u(Ohmb=}r(uPI( zYFkRkPEM?_6c$!#R9`A!Z&$p!rWWfeoVn3RE)!E*{qEYr>OFY@;o{V#$&(}1{MVQah zd&!Y+D?CMO@@8_n;4)4 zYeUabF2Mj5rhfFj-`Y)H(S&xd*=|c6vI@%1o1jZ6TgOEi{)k>DC-Ve$Mw;a85iDAa zrIOn1bvWkpV5S1tb}3A@2}aw4o!$?FTt#zma^})8pw*2)qmRO5cT<5fWwG*j!ogiE zy70F|oih2h6yG$+;&Q^;yI;V#@_x0y!n?|cQz8H~k1tT4W%wN93f~B?HqoeNgx8wi zHjnYeCvixN3dXp4n5wYFmnQhyVOpAW?Qv@JHC2pqee$TWc!KHR%PDI@;18F#o7CzB zaPVCW0gDjp^t7@?0X%LYY*B@|ZiL%h3VsLd(MrxqYc>H9J*kY!AuoSksvynn;0E4i zt@$Eok!$eW#rpLpNk79)E#8V%W87l;s6b*i@vG3C0A3&%tcLq9Q$zMiV5SrEd)ezV zjm51ayvtW@J4f?9wucx$Oi(VWXAP6hb~1rFaEx)<<@1j66{dgi>2W^bgU5PoqkPo} z_uM&2bhn5G6QYGMDuw`h`KoiyaBsEFSsU!*wAWYd@MMqjA&=gbZ}&J%MEe*$S9F)) z*tf@dABNC9HcyU67r}ltk1Kpdg`*rXwU6;2R$x?$Y_(kp;c%U#iz;!r)Ka`2yJ%nl z6%HF8#wqK{wl^(3clLLH}wI1+!D@XaQ=8Q)8hB1D}@Wcxq`v~7O zj9+&pj?8TpXZZH4F1KCq(|fwfV{aX%A(K=!d312SCRt{x#{$mq2h6FPYENVAx~9sc zMlVM9i}^lmYJEaXQCw*b+>{ba8<Tg)hTJ zuRIqj{93wH@j`{K#cze=xlrME;8!%l3l+WtR~y0$75+;6ph9?|!iQ;4@j`_+=&<62 z3V#egG!S{A!oQu4D_*GZC+JSa3l;tp4J%%#@Smj-#S0bwA$pn*L7&h7_-E+5iWe&U zv-G^;g$n;X-mqN>d7-<(zeGP*yinm^!OJCiE>!q`@*Kqr1&@~+g~BT;{9<0Oc%j0t z$My1NKEc=UEiz9;C+#I@xBjF>plbp+<4|yB zid#xj9Uzm|1xgSWr48y&6D^0L+wRozj5x z7SwFTxJ#2FU{oTQm;uD@u*7oY5vF#BIX?xChY+45CCLYHmtncJ5NW|OIxxclfaED0 zjfW7RoHHuDx7j%)C=7RG0V}_RWFeQH{5Y+! zl$eMY^yMpzeazAV#QDnULVP|^jW}>i(MdWE7|yQrNWvmGE@+Ndoq(&50^}!?fLslj z{0c^E_1Mk+3Xuyv_5>umfwcAn@NO{i9l+bnS)0eHUrp9yALXYjx{5qG=6uHZo5K^| z^Vk%9#$%J|DbQQyu}uTY-$?`IVcON4!q*B+c);gHWdB|o!0MvUW1FmT651v;CeU`; zE|Z`)*~R-PcD)*&vK_7~v<@zC8Esd*P~k7fnsSy0!Z*}lWx|@6|Ds;365L!ymGRtZ~({WQ`GAv5+mK>7rkRc~2#q5ES zNUT$G&}t@qrcPakg|!kehwcjIWYMU#EsbbO6Nhj*^j!lOlNw3~^yNXhz-%?9UXQ$e z7l7vgx#?Pjk+&gZeLtR@K{Yl?Tj^n{qc2119DYLbA~n-*@EY8K_j0-TO=$t%R;<9C z{whFUBOLS)pzjD`{`L5s9^zjxRgCcOkPZI<3H6^w`OlA{k69!j z75@jVP|-#vhxZsLdNkArS+ru>yNLoMUFFv0%Tv~}dvK3%U8<1ToGfIvAp5WQ6fGF1 z8+?nA!~VSq|GizK{Mu>n7{BhT=&BgeXiU>;Mzm~@O-is3bq~F!q!lqcB&Uop{yM4! zvvT&T04eo@hU(6SYtJ02fn~Si>@?!pF8Ito_{>!_TUk1BOOmQ(q=I3s66!Vn3#^OZ zl3L30-u(Lz$f!>_Ks)^WSt|L2Ly{9 zrX_MMgiVpB4{Blwkfr9+ifK1y9%a2obrYBniZ?y_iyMI|v6eS0nHv*;BTMVg4eMbu?czeNKU4O+C< zq9Kd+TXevpS)dx$Q3ZMyD-uX`TtQiIjz*f*2-le1h Date: Wed, 11 May 2022 16:42:19 +0200 Subject: [PATCH 2/2] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e086126a..c974c52d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +* Ignore R8 minified libs from instrumentation (#316) + ## 3.1.0-beta.1 * Fix: obfuscated libs instrumentation (adcolony) (#307)