Source: libgpod Severity: important Tags: patch upstream Version: 0.8.3-1
Dear maintainer, TL;DR: LP: #1221031 was never fixed in Debian, please fix it. Patch attached at the bottom. Now down to the detailed report, provided by GPT 6.1 Sol: The iphone-set-info udev helper passes an uninitialized service descriptor pointer to lockdownd_start_service(). This is undefined behaviour and can cause the helper to crash or write through an unintended address when it tries to open the AFC service. In tools/ipod-lockdown.c, iphone_write_sysinfo_extended() declares: lockdownd_service_descriptor_t service; and later calls: lockdownd_start_service(client, "com.apple.afc", &service) without first assigning service. libimobiledevice supports descriptor reuse: when *service is non-NULL, it resets (*service)->port and ssl_enabled before sending a request. Consequently, it reads and may dereference the uninitialized pointer supplied by libgpod. The variable must be initialized to NULL when requesting a newly allocated descriptor. I checked the archived Debian amd64 binary libgpod-common 0.8.3-23+b1. The omission is also present in /usr/lib/udev/iphone-set-info, rather than being confined to the upstream source. In the relevant function, the client, device and afc pointer slots at rsp+0x08, +0x10 and +0x18 are initialized, but the service slot at rsp+0x20 is not written before this call: 17c2: lea 0x20(%rsp),%rdx 17c7: lea ...,%rsi # "com.apple.afc" 17ce: call lockdownd_start_service@plt In the archived libimobiledevice-1.0-6 1.4.0-1 amd64 binary, the exported lockdownd_start_service entry at 0x124b0 passes this pointer argument to the internal implementation at 0x106c0. That implementation contains: 106fd: mov (%rcx),%rax # load *service 10706: test %rax,%rax 10709: je 10714 # skip reset when NULL 1070d: movb $0x0,0x2(%rax) # reset ssl_enabled 10711: mov %dx,(%rax) # reset port The faulting instruction bytes in Debian #1120105 match the write at 0x1070d. This provides a strong explanation for the iphone-set-info crashes in that report. However, the reporter's libgpod version is unspecified, and I have not established a causal connection to its additional desktop input-device symptoms: https://bugs.debian.org/1120105 The same uninitialized declaration remains in the upstream libgpod master (8dc5015ae036b219c4c9579a156886aa3a722aa5). It was introduced by the 2013 libimobiledevice API migration, e620b2fbdd818a4b32fa927875936ba0476952e5. The library's descriptor-reuse behaviour is also present in libimobiledevice 1.3.0 and its current master; this is not a newly introduced 1.4.0 behaviour. https://sourceforge.net/p/gtkpod/libgpod/ci/e620b2fbdd818a4b32fa927875936= ba0476952e5/ Existing reports were checked before preparing this report: * Debian #758099 also describes an iphone-set-info crash. Its log shows a NULL read in libc, without a backtrace establishing this descriptor bug, and it refers to a historical usbmuxd issue. I cannot establish that it is a duplicate of the uninitialized-pointer defect reported here. https://bugs.debian.org/758099 * Ubuntu LP #1221031 reports a crash after the libimobiledevice API-migration patch had been applied. Ubuntu fixed it in libgpod 0.8.2-7ubuntu4 by adding exactly this NULL initialization to tools/ipod-lockdown.c, and the SRU verification was completed successfully. I checked the actual patch in the archived Ubuntu source package. This is a historical fix for the same defect, which is absent from the Debian source and binary checked here. https://bugs.launchpad.net/bugs/1221031 The historical Ubuntu fix should not be mistaken for a fix in the current source: Ubuntu's published 0.8.3-23 source uses the same Debian tarball as Debian, and its patch series does not include this initialization. The loss can be traced to Ubuntu 0.8.3-0ubuntu1: its upstream-version update removed the entire API-migration patch as "Applied upstream", although upstream 0.8.3 did not include the extra NULL initialization. The subsequent 0.8.3-1.1ubuntu1 Debian merge (LP #1259706) described the patch as "Dropped, fixed upstream" and did not restore the initialization. I applied all patches from both archived source versions and confirmed that their effective tools/ipod-lockdown.c leaves service uninitialized. https://launchpad.net/ubuntu/+source/libgpod/0.8.3-0ubuntu1 https://bugs.launchpad.net/bugs/1259706 * Ubuntu LP #1212546 concerns the preceding API mismatch, fixed by importing e620b2f in 0.8.2-7ubuntu3. That migration still omitted this initialization; LP #1221031 records the subsequent correction. These two fixes should not be conflated. https://bugs.launchpad.net/bugs/1212546 The attached patch makes the single-line change to initialize service to NULL. It applies with fuzz=3D0 to libgpod 0.8.3-23 after all eight existing Debian patches have been applied. The resulting source differs only in that initialization. This report is based on source and binary analysis. I do not have an iPhone available, and have not performed a dynamic reproduction, package rebuild or hardware test. Severity important is requested because the defect can break the helper's AFC/SysInfoExtended operation. Regards, Boyuan Yang diff --git a/tools/ipod-lockdown.c b/tools/ipod-lockdown.c --- a/tools/ipod-lockdown.c +++ b/tools/ipod-lockdown.c @@ -157,7 +157,7 @@ idevice_error_t ret = IDEVICE_E_UNKNOWN_ERROR; afc_error_t afc_ret; #ifdef HAVE_LIBIMOBILEDEVICE_1_1_5 - lockdownd_service_descriptor_t service; + lockdownd_service_descriptor_t service = NULL; #else uint16_t afcport = 0; #endif
OpenPGP_signature.asc
Description: OpenPGP digital signature
