objective-see / BlockBlock

BlockBlock provides continual protection by monitoring persistence locations.
GNU General Public License v3.0
619 stars 38 forks source link

Crash on parsing an invalid plist file #46

Closed xnyhps closed 1 year ago

xnyhps commented 2 years ago

The function -[Launchd itemObject:] attempts to find the executable by parsing the plist of a newly installed launch agent. The checks on the parsed data are lacking, making it possible to crash the BlockBlock service, thereby bypassing its checks.

https://github.com/objective-see/BlockBlock/blob/7e2712e7aeb6a32e3b6b575dde7840f5d3d1ab7e/Daemon/Daemon/Plugins/Launchd.m#L108-L140

The check to make sure that itemBinary is an NSString on line 125 is performed after already calling -length on it on line 118. In addition, the goto bail on line 121 does not nil itemBinary, which means that it will be returned, but without checking its class.

By providing a launchd configuration where Program is an NSData object of length 0 (which also responds to -length), the check on line 118 will cause it to go to bail, which means that the empty NSData object is returned. Then on line 75, -lastPathComponent will be called on an NSData object, causing an exception.

For example, the following plist file:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>Label</key>
    <string>asdf</string>
    <key>Program</key>
    <data></data>
</dict>
</plist>

Triggers this exception:

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[__NSCFData lastPathComponent]: unrecognized selector sent to instance 0x60000384f600'
*** First throw call stack:
(
    0   CoreFoundation                      0x000000018f929198 __exceptionPreprocess + 240
    1   libobjc.A.dylib                     0x000000018f673e04 objc_exception_throw + 60
    2   CoreFoundation                      0x000000018f9bcf40 -[NSObject(NSObject) __retain_OA] + 0
    3   CoreFoundation                      0x000000018f888544 ___forwarding___ + 1764
    4   CoreFoundation                      0x000000018f887da0 _CF_forwarding_prep_0 + 96
    5   BlockBlock                          0x0000000102051414 BlockBlock + 37908
    6   BlockBlock                          0x0000000102049768 BlockBlock + 5992
    7   BlockBlock                          0x000000010204c248 BlockBlock + 16968
    8   BlockBlock                          0x0000000102052da4 BlockBlock + 44452
    9   BlockBlock                          0x00000001020<…>

When the BlockBlock service crashes, it stops checking for new or modified launch agents. Unless the user is looking at the Console logs carefully, it is not possible to tell that BlockBlock has crashed. The menu item keeps working and no crash reporter opens.

Recommendation

I recommend:

objective-see commented 1 year ago

Mahalo for the detailed bug report 🙏 ...updated the code to fix this issue!

objective-see commented 1 year ago

Fixed in v2.1.5 (https://github.com/objective-see/BlockBlock/commit/c5297257ed99a9f496b45fc71ba8eeab86af502b) ☺️