From d6609976f4b96f8241d1223315f1ddc6fe0fbc17 Mon Sep 17 00:00:00 2001 From: Hristo Staykov Date: Mon, 4 Mar 2024 07:56:11 -0800 Subject: [PATCH] Fix race condition in CFPreferences In __CFWriteBytesToFileWithAtomicity(), we first write the contents of the plist to an auxiliary copy file, and then move that copy to where the original used to be. Because that file needs to have the same owner as the original, we would use chown() to change ownership as the last step. This allows a race condition where the new file is in its final location, but doesn't have the correct permissions. To fix this, call chown() on the file before moving. rdar://121597642 --- .../Preferences.subproj/CFXMLPreferencesDomain.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/CoreFoundation/Preferences.subproj/CFXMLPreferencesDomain.c b/CoreFoundation/Preferences.subproj/CFXMLPreferencesDomain.c index c7a87de559..1b98874c62 100644 --- a/CoreFoundation/Preferences.subproj/CFXMLPreferencesDomain.c +++ b/CoreFoundation/Preferences.subproj/CFXMLPreferencesDomain.c @@ -273,6 +273,10 @@ static Boolean __CFWriteBytesToFileWithAtomicity(CFURLRef url, const void *bytes close(fd); if (atomic) { + // If the file was renamed successfully and we wrote it as root we need to reset the owner & group as they were. + if (writingFileAsRoot) { + chown(auxPath, owner, group); + } // preserve the mode as passed in originally chmod(auxPath, mode); @@ -280,11 +284,6 @@ static Boolean __CFWriteBytesToFileWithAtomicity(CFURLRef url, const void *bytes unlink(auxPath); return false; } - - // If the file was renamed successfully and we wrote it as root we need to reset the owner & group as they were. - if (writingFileAsRoot) { - chown(cpath, owner, group); - } } return true; }