I’ve just released the security update Vienna 2.3.2. Security is no laughing matter, so all Vienna users need to update to version 2.3.2. Release notes are here.
The technical details of the vulnerability are instructive. The problem was this line of code:
returnCode = NSRunAlertPanel(alertTitle, alertBody, NSLocalizedString(@"Delete", nil), NSLocalizedString(@"Cancel", nil), nil);
Well, that’s not entirely true. The problem was that line combined with certain assignments of
alertBody, for example:
alertBody = [NSString stringWithFormat:@"Are you sure you want to unsubscribe from \"%@\"? This operation will delete all cached articles.", [folder name]];
By default, the value of
[folder name] comes from a most trustworthy source: the internet. If, by chance or design, the name contains conversion specifiers, you’re in big trouble. For a discussion of this kind of vulnerability, see Apple’s Secure Coding Guide. The Secure Coding Guide is a must read for Mac Developers. (I also recommend this. Tell them Jeff sent you.)
One simple fix for the vulnerability is as follows:
returnCode = NSRunAlertPanel(alertTitle, @"%@", NSLocalizedString(@"Delete", nil), NSLocalizedString(@"Cancel", nil), nil, alertBody);
With this fix, the
alertBody string — which contains the unvalidated folder name — is no longer treated as a format string.
You need to be very careful about using
NSRunAlertPanel() and similar functions. Unfortunately, the API and documentation are quite bad. The Discussion doesn’t even mention that the second argument is a format string; for that, you have to read the documentation for another function. Furthermore, it’s absolutely insane to put three unrelated items between the format string and the optional arguments for the format string.
As you know, however, you go to war with the API you have, not the API you might want or wish to have at a later time.