[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
MiNT 1.09 security: revised f_delete() patch + more
Bonjour Thierry!
> Hmm, if you want to support the sticky bit, you must also patch the
> f_rename() and f_rmdir() functions, because you shouldn't be allowed to
> rename/move files and subdirectories of a sticky folder. (Otherwise, it
> would be a big security hole: you could move files to a non-sticky
> folder, and delete them.)
So it wasn't directly wrong, but just incomplete. Thanx for this info,
you're of course right! This should be fixed with this revised patch,
which is again relative to the original code because:
1) everyone should have made a backup before
2) everyone should know how to unpatch a previous patch
3) probably nobody has applied the old one anyway ;-)
Now f_delete(), f_rename() and d_delete() are patched so that you may
not do an operation you want if "the sticky bit is set, and you're not
superuser, and the item doesn't belong to you, and - in addition to your
ramfs - the directory doesn't belong to you."
You could in any case change the filemode of the directory if it belongs
to you and then do what you like, but it seems more logical and/or convenient
to me to do it this way.
I've also changed the d_setpath() call so that it is now no longer allowed
to change to a directory you don't have permission to. Until now, you can
_change_ to any directory you like, but maybe refused to read it later. So
this isn't a big problem, but it looks to be a bit more un*x like. :-)
Je yous souhaites un bon joel, un tres bien annee' nouvaux et peut-`etre
aussi des bon vacances d'hiver, si vous aller faire du ski ou quelque
chose comme ,ca.
Oh no, my french... :-(
anyway, happy X-Mas and a jolly new year 4 all :-)
TeSche
--
PS: If the above written looks weird, then that's probably because it _is_.
WhoDunnIt: Torsten Scherer (Schiller, Tesche, ...)
Where: Faculty of Technology, University of Bielefeld, Germany
EMail: itschere@techfak.uni-bielefeld.de / tesche@hrz.uni-bielefeld.de
======= cut cut cut =======
diff -u5 ./dosdir.c ../my/dosdir.c
--- ./dosdir.c Fri Nov 19 15:34:12 1993
+++ ../my/dosdir.c Sun Dec 19 14:19:58 1993
@@ -113,11 +113,11 @@
if (r) {
DEBUG(("Dcreate(%s): returning %ld", path, r));
return r; /* an error occured */
}
/* check for write permission on the directory */
- r = dir_access(&dir, S_IWOTH);
+ r = dir_access(&dir, S_IWOTH, 0);
if (r) {
DEBUG(("Dcreate(%s): access to directory denied",path));
release_cookie(&dir);
return r;
}
@@ -124,10 +124,13 @@
r = (*dir.fs->mkdir)(&dir, temp1, DEFAULT_DIRMODE & ~curproc->umask);
release_cookie(&dir);
return r;
}
+/* tesche: delete a directory with respect of the sticky bit
+ */
+
long ARGS_ON_STACK
d_delete(path)
const char *path;
{
fcookie parentdir, targdir;
@@ -134,10 +137,11 @@
long r;
PROC *p;
int i;
XATTR xattr;
char temp1[PATH_MAX];
+ short sticky_uid;
TRACE(("Ddelete(%s)", path));
r = path2cookie(path, temp1, &parentdir);
@@ -147,18 +151,17 @@
return r;
}
/* check for write permission on the directory which the target
* is located
*/
- if ((r = dir_access(&parentdir, S_IWOTH)) != 0) {
+ if ((r = dir_access(&parentdir, S_IWOTH, &sticky_uid))) {
DEBUG(("Ddelete(%s): access to directory denied", path));
release_cookie(&parentdir);
return r;
}
/* now get the info on the file itself */
-
r = relpath2cookie(&parentdir, temp1, NULL, &targdir, 0);
if (r) {
bailout:
release_cookie(&parentdir);
DEBUG(("Ddelete: error %ld on %s", r, path));
@@ -167,10 +170,20 @@
if ((r = (*targdir.fs->getxattr)(&targdir, &xattr)) != 0) {
release_cookie(&targdir);
goto bailout;
}
+/* check for sticky bit */
+ if ((sticky_uid >= 0) &&
+ curproc->euid &&
+ (sticky_uid != curproc->euid) &&
+ (curproc->euid != xattr.uid)) {
+ release_cookie(&targdir);
+ release_cookie(&parentdir);
+ return EACCDN;
+ }
+
/* if the "directory" is a symbolic link, really unlink it */
if ( (xattr.mode & S_IFMT) == S_IFLNK ) {
r = (*parentdir.fs->remove)(&parentdir, temp1);
} else if ( (xattr.mode & S_IFMT) != S_IFDIR ) {
DEBUG(("Ddelete: %s is not a directory", path));
@@ -215,11 +228,10 @@
fcookie dir;
int drv = curproc->curdrv;
int i;
char c;
long r;
- XATTR xattr;
TRACE(("Dsetpath(%s)", path));
r = path2cookie(path, follow_links, &dir);
@@ -234,24 +246,16 @@
drv = c-'a';
else if (c >= 'A' && c <= 'Z')
drv = c-'A';
}
- r = (*dir.fs->getxattr)(&dir, &xattr);
-
- if (r < 0) {
- DEBUG(("Dsetpath: file '%s': attributes not found", path));
+ if ((r = dir_access(&dir, S_IXOTH, 0))) {
+ DEBUG(("Dsetpath(%s): not a directory or access denied", path));
release_cookie(&dir);
return r;
}
- if (!(xattr.attr & FA_DIR)) {
- DEBUG(("Dsetpath(%s): not a directory",path));
- release_cookie(&dir);
- return EPTHNF;
- }
-
/*
* watch out for symbolic links; if c:\foo is a link to d:\bar, then
* "cd c:\foo" should also change the drive to d:
*/
if (drv != UNIDRV && dir.dev != curproc->root[drv].dev) {
@@ -504,11 +508,11 @@
}
/* check to see if we have read permission on the directory (and make
* sure that it really is a directory!)
*/
- r = dir_access(&dir, S_IROTH);
+ r = dir_access(&dir, S_IROTH, 0);
if (r) {
DEBUG(("Fsfirst(%s): access to directory denied (error code %ld)", path, r));
release_cookie(&dir);
return r;
}
@@ -716,48 +720,88 @@
release_cookie(&fc);
return xattr.attr;
}
}
+/* tesche: delete a file with respect of the sticky bit
+ */
+
long ARGS_ON_STACK
f_delete(name)
const char *name;
{
- fcookie dir;
- long r;
- char temp1[PATH_MAX];
+ fcookie dir, fc;
+ long r;
+ char temp1[PATH_MAX];
+ XATTR fxattr;
+ short sticky_uid;
TRACE(("Fdelete(%s)", name));
- r = path2cookie(name, temp1, &dir);
-
- if (r) {
- DEBUG(("Fdelete: error %ld", r));
+/* first, get the directory cookie */
+ if ((r = path2cookie(name, temp1, &dir))) {
+ DEBUG(("Fdelete(%s): directory not found", name));
return r;
}
/* check for write permission on directory */
- r = dir_access(&dir, S_IWOTH);
- if (r) {
- DEBUG(("Fdelete(%s): write access to directory denied",name));
- } else {
-/* BUG: we should check here for a read-only file */
- r = (*dir.fs->remove)(&dir,temp1);
+ if (dir_access(&dir, S_IWOTH, &sticky_uid)) {
+ DEBUG(("Fdelete(%s): write access to directory denied", name));
+ release_cookie(&dir);
+ return EACCDN;
}
+
+/* if (sticky) and not(superuser) and not(my_directory): need more checks */
+ if ((sticky_uid >= 0) &&
+ curproc->euid &&
+ (sticky_uid != curproc->euid)) {
+ if ((r = relpath2cookie(&dir, temp1, 0, &fc, 0))) {
+ DEBUG(("Fdelete(%s): error %ld, perhaps file not found", name, r));
+ release_cookie(&dir);
+ return r;
+ }
+
+ /* check ownership of the file */
+ if ((r = (fc.fs->getxattr)(&fc, &fxattr))) {
+ DEBUG(("Fdelete(%s): can't get file attributes", name));
+ release_cookie(&fc);
+ release_cookie(&dir);
+ return r;
+ }
+
+ if (curproc->euid != fxattr.uid) {
+ DEBUG(("Fdelete(%s): write access to file denied", name));
+ release_cookie(&fc);
+ release_cookie(&dir);
+ return EACCDN;
+ }
+
+ release_cookie(&fc);
+ } /* sticky */
+
+/* actually delete the file */
+ if ((r = (*dir.fs->remove)(&dir,temp1)))
+ DEBUG(("Fdelete(%s): error %ld", name, r));
+
release_cookie(&dir);
+
return r;
}
+/* tesche: rename/move a file with respect of the sticky bit
+ */
+
long ARGS_ON_STACK
f_rename(junk, old, new)
int junk; /* ignored, for TOS compatibility */
const char *old, *new;
{
fcookie olddir, newdir, oldfil;
XATTR xattr;
char temp1[PATH_MAX], temp2[PATH_MAX];
long r;
+ short sticky_uid;
UNUSED(junk);
TRACE(("Frename(%s, %s)", old, new));
@@ -799,12 +843,21 @@
release_cookie(&newdir);
return EXDEV; /* cross device rename */
}
/* check for write permission on both directories */
- r = dir_access(&olddir, S_IWOTH);
- if (!r) r = dir_access(&newdir, S_IWOTH);
+ if (!(r = dir_access(&olddir, S_IWOTH, &sticky_uid)))
+ r = dir_access(&newdir, S_IWOTH, 0);
+
+/* check for sticky bit */
+ if (!r &&
+ (sticky_uid >= 0) &&
+ curproc->euid &&
+ (sticky_uid != curproc->euid) &&
+ (xattr.uid != curproc->euid)) /* oldfil specs */
+ r = EACCDN;
+
if (r) {
DEBUG(("Frename(%s,%s): access to a directory denied",old,new));
} else {
r = (*newdir.fs->rename)(&olddir, temp2, &newdir, temp1);
}
@@ -873,11 +926,11 @@
r = path2cookie(name, follow_links, &dir);
if (r) {
DEBUG(("Dopendir(%s): error %ld", name, r));
return r;
}
- r = dir_access(&dir, S_IROTH);
+ r = dir_access(&dir, S_IROTH, 0);
if (r) {
DEBUG(("Dopendir(%s): read permission denied", name));
release_cookie(&dir);
return r;
}
@@ -1042,11 +1095,11 @@
return EXDEV; /* cross device link */
}
/* check for write permission on the destination directory */
- r = dir_access(&newdir, S_IWOTH);
+ r = dir_access(&newdir, S_IWOTH, 0);
if (r) {
DEBUG(("Flink(%s,%s): access to directory denied",old,new));
} else
r = (*newdir.fs->hardlink)(&olddir, temp2, &newdir, temp1);
release_cookie(&olddir);
@@ -1072,11 +1125,11 @@
r = path2cookie(new, temp1, &newdir);
if (r) {
DEBUG(("Fsymlink(%s,%s): error parsing %s", old,new,new));
return r;
}
- r = dir_access(&newdir, S_IWOTH);
+ r = dir_access(&newdir, S_IWOTH, 0);
if (r) {
DEBUG(("Fsymlink(%s,%s): access to directory denied",old,new));
} else
r = (*newdir.fs->symlink)(&newdir, temp1, old);
release_cookie(&newdir);
diff -u5 ./filesys.c ../my/filesys.c
--- ./filesys.c Fri Nov 19 15:34:22 1993
+++ ../my/filesys.c Sat Dec 18 14:41:38 1993
@@ -1100,16 +1100,18 @@
return t;
}
/*
* check to see that a file is a directory, and that write permission
- * is granted; return an error code, or 0 if everything is ok.
+ * is granted; return an error code, or 0 if everything is ok. also return
+ * the uid of the directory if the sticky bit is set, or -1 otherwise,
*/
long
-dir_access(dir, perm)
+dir_access(dir, perm, sticky_uid)
fcookie *dir;
unsigned perm;
+ short *sticky_uid;
{
XATTR xattr;
long r;
r = (*dir->fs->getxattr)(dir, &xattr);
@@ -1122,10 +1124,16 @@
return EPTHNF;
}
if (denyaccess(&xattr, perm)) {
DEBUG(("no permission for directory"));
return EACCDN;
+ }
+ if (sticky_uid) {
+ if (xattr.mode & S_ISVTX) {
+ *sticky_uid = xattr.uid;
+ } else
+ *sticky_uid = -1;
}
return 0;
}
/*
diff -u5 ./proto.h ../my/proto.h
--- ./proto.h Fri Nov 19 15:34:38 1993
+++ ../my/proto.h Sat Dec 18 14:42:16 1993
@@ -182,11 +182,11 @@
FILEPTR *new_fileptr P_((void));
void dispose_fileptr P_((FILEPTR *f));
int ARGS_ON_STACK denyshare P_((FILEPTR *list, FILEPTR *newfileptr));
int denyaccess P_((XATTR *, unsigned));
LOCK * ARGS_ON_STACK denylock P_((LOCK *list, LOCK *newlock));
-long dir_access P_((fcookie *, unsigned));
+long dir_access P_((fcookie *, unsigned, short *));
int has_wild P_((const char *name));
void copy8_3 P_((char *dest, const char *src));
int pat_match P_((const char *name, const char *template));
int samefile P_((fcookie *, fcookie *));