Fix the WriteStringToFile overload that takes mode/owner/group.
The actual bug is == instead of !=, but the real cause was me trying to be too clever. This patch switches to much simpler code, and -- since the intended use of this code is security anyway -- adds logging if anything goes wrong. Bug: 19361774 Change-Id: If2af07d31a5002f9010b838247b691f6b28bdfb1
This commit is contained in:
parent
29576ae890
commit
9d1f515ed1
2 changed files with 31 additions and 2 deletions
|
@ -14,6 +14,9 @@
|
|||
* limitations under the License.
|
||||
*/
|
||||
|
||||
#define LOG_TAG "utils.file"
|
||||
#include <cutils/log.h>
|
||||
|
||||
#include "utils/file.h"
|
||||
|
||||
#include <errno.h>
|
||||
|
@ -75,14 +78,26 @@ bool android::WriteStringToFile(const std::string& content, const std::string& p
|
|||
O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,
|
||||
mode));
|
||||
if (fd == -1) {
|
||||
ALOGE("android::WriteStringToFile open failed: %s", strerror(errno));
|
||||
return false;
|
||||
}
|
||||
|
||||
// We do an explicit fchmod here because we assume that the caller really meant what they
|
||||
// said and doesn't want the umask-influenced mode.
|
||||
bool result = (fchmod(fd, mode) != -1 && fchown(fd, owner, group) == -1 && WriteStringToFd(content, fd));
|
||||
if (fchmod(fd, mode) == -1) {
|
||||
ALOGE("android::WriteStringToFile fchmod failed: %s", strerror(errno));
|
||||
return CleanUpAfterFailedWrite(path);
|
||||
}
|
||||
if (fchown(fd, owner, group) == -1) {
|
||||
ALOGE("android::WriteStringToFile fchown failed: %s", strerror(errno));
|
||||
return CleanUpAfterFailedWrite(path);
|
||||
}
|
||||
if (!WriteStringToFd(content, fd)) {
|
||||
ALOGE("android::WriteStringToFile write failed: %s", strerror(errno));
|
||||
return CleanUpAfterFailedWrite(path);
|
||||
}
|
||||
TEMP_FAILURE_RETRY(close(fd));
|
||||
return result || CleanUpAfterFailedWrite(path);
|
||||
return true;
|
||||
}
|
||||
#endif
|
||||
|
||||
|
|
|
@ -71,6 +71,20 @@ TEST(file, WriteStringToFile) {
|
|||
EXPECT_EQ("abc", s);
|
||||
}
|
||||
|
||||
TEST(file, WriteStringToFile2) {
|
||||
TemporaryFile tf;
|
||||
ASSERT_TRUE(tf.fd != -1);
|
||||
ASSERT_TRUE(android::WriteStringToFile("abc", tf.filename, 0660, getuid(), getgid())) << errno;
|
||||
struct stat sb;
|
||||
ASSERT_EQ(0, stat(tf.filename, &sb));
|
||||
ASSERT_EQ(0660U, (sb.st_mode & ~S_IFMT));
|
||||
ASSERT_EQ(getuid(), sb.st_uid);
|
||||
ASSERT_EQ(getgid(), sb.st_gid);
|
||||
std::string s;
|
||||
ASSERT_TRUE(android::ReadFileToString(tf.filename, &s)) << errno;
|
||||
EXPECT_EQ("abc", s);
|
||||
}
|
||||
|
||||
TEST(file, WriteStringToFd) {
|
||||
TemporaryFile tf;
|
||||
ASSERT_TRUE(tf.fd != -1);
|
||||
|
|
Loading…
Reference in a new issue