Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix umask handling when creating new files
  * src/transform.c (transform_save): faulty umask arithmetic would cause
    overly-open file modes when the umask contains "7", as the umask was
    incorrectly subtracted from the target file mode

Fixes CVE-2013-6412, RHBZ#1034261
  • Loading branch information
Dominic Cleal authored and lutter committed Jan 14, 2014
1 parent b88232b commit f5b4fc0
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/transform.c
Expand Up @@ -1144,7 +1144,7 @@ int transform_save(struct augeas *aug, struct tree *xfm,
mode_t curumsk = umask(022);
umask(curumsk);

if (fchmod(fileno(fp), 0666 - curumsk) < 0) {
if (fchmod(fileno(fp), 0666 & ~curumsk) < 0) {
err_status = "create_chmod";
return -1;
}
Expand Down
48 changes: 48 additions & 0 deletions tests/test-save.c
Expand Up @@ -26,6 +26,7 @@
#include "cutest.h"

#include <stdio.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/wait.h>

Expand All @@ -51,6 +52,7 @@ static void setup(CuTest *tc) {
if (asprintf(&lensdir, "%s/lenses", abs_top_srcdir) < 0)
CuFail(tc, "asprintf lensdir failed");

umask(0022);
run(tc, "test -d %s && chmod -R u+w %s || :", root, root);
run(tc, "rm -rf %s", root);
run(tc, "mkdir -p %s", root);
Expand Down Expand Up @@ -221,6 +223,49 @@ static void testDoubleSlashPath(CuTest *tc) {
CuAssertIntEquals(tc, 1, r);
}

/* Check the umask is followed when creating files
*/
static void testUmask(CuTest *tc, int tumask, mode_t expected_mode) {
int r;
struct stat buf;
char* fpath = NULL;

if (asprintf(&fpath, "%s/etc/test", root) < 0) {
CuFail(tc, "failed to set root");
}

umask(tumask);

r = aug_rm(aug, "/augeas/load/*");
CuAssertPositive(tc, r);

r = aug_set(aug, "/augeas/load/Test/lens", "Simplelines.lns");
CuAssertRetSuccess(tc, r);
r = aug_set(aug, "/augeas/load/Test/incl", "/etc/test");
CuAssertRetSuccess(tc, r);
r = aug_load(aug);
CuAssertRetSuccess(tc, r);
r = aug_set(aug, "/files/etc/test/1", "test");
CuAssertRetSuccess(tc, r);

r = aug_save(aug);
CuAssertRetSuccess(tc, r);
r = aug_match(aug, "/augeas//error", NULL);
CuAssertIntEquals(tc, 0, r);

CuAssertIntEquals(tc, 0, stat(fpath, &buf));
CuAssertIntEquals(tc, expected_mode, buf.st_mode & 0777);
}
static void testUmask077(CuTest *tc) {
testUmask(tc, 0077, 0600);
}
static void testUmask027(CuTest *tc) {
testUmask(tc, 0027, 0640);
}
static void testUmask022(CuTest *tc) {
testUmask(tc, 0022, 0644);
}

int main(void) {
char *output = NULL;
CuSuite* suite = CuSuiteNew();
Expand All @@ -245,6 +290,9 @@ int main(void) {
SUITE_ADD_TEST(suite, testMtime);
SUITE_ADD_TEST(suite, testRelPath);
SUITE_ADD_TEST(suite, testDoubleSlashPath);
SUITE_ADD_TEST(suite, testUmask077);
SUITE_ADD_TEST(suite, testUmask027);
SUITE_ADD_TEST(suite, testUmask022);

CuSuiteRun(suite);
CuSuiteSummary(suite, &output);
Expand Down

0 comments on commit f5b4fc0

Please sign in to comment.