Okay, forget O_EXCL, do lots of other obscure checks instead =)

git-svn-id: svn://svn.savannah.gnu.org/nano/trunk/nano@381 35c25a1d-7b9e-4130-9fde-d3aeb78583b8
master
Chris Allegretta 2000-12-04 05:15:39 +00:00
parent 908805a2e8
commit 07f9ee059f
2 changed files with 31 additions and 18 deletions

View File

@ -1,9 +1,9 @@
CVS code - CVS code -
- files.c: - files.c:
write_file() write_file()
- Added O_EXCL to open call if tmp is set, more security which hopefully
fixes any remaining security issues.
- Added tmp check to TMP_OPT section (how apropriate). - Added tmp check to TMP_OPT section (how apropriate).
- Added new consistency checking code from securityfocus
article by Oliver Friedrichs.
- winio.c: - winio.c:
edit_add() edit_add()
- Off by one display error (fix by Rocco Corsi). - Off by one display error (fix by Rocco Corsi).

45
files.c
View File

@ -301,8 +301,8 @@ int write_file(char *name, int tmp)
long size, lineswritten = 0; long size, lineswritten = 0;
char buf[PATH_MAX + 1]; char buf[PATH_MAX + 1];
filestruct *fileptr; filestruct *fileptr;
int fd, mask = 0, realexists; int fd, mask = 0, realexists, anyexists;
struct stat st; struct stat st, st2;
static char *realname = NULL; static char *realname = NULL;
if (!strcmp(name, "")) { if (!strcmp(name, "")) {
@ -327,24 +327,18 @@ int write_file(char *name, int tmp)
/* Check to see if the file is a regular file and FOLLOW_SYMLINKS is /* Check to see if the file is a regular file and FOLLOW_SYMLINKS is
set. If so then don't do the delete and recreate code which would set. If so then don't do the delete and recreate code which would
cause unexpected behavior */ cause unexpected behavior */
lstat(realname, &st); anyexists = lstat(realname, &st);
/* New case: if the file exists, just give up. Easy way out of /* New case: if the file exists, just give up */
all security issues */ if (tmp && anyexists != -1)
if (tmp && realexists != -1)
return -1; return -1;
else if (ISSET(FOLLOW_SYMLINKS) || !S_ISLNK(st.st_mode)) { else if (ISSET(FOLLOW_SYMLINKS) || !S_ISLNK(st.st_mode)) {
/* If tmp is set, use O_EXCL, more security, YAY! */ fd = open(realname, O_WRONLY | O_CREAT | O_TRUNC,
if (tmp) S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
fd = open(realname, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC, S_IWOTH);
S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
S_IWOTH); /* First, just give up if we couldn't even open the file */
else
fd = open(realname, O_WRONLY | O_CREAT | O_TRUNC,
S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH |
S_IWOTH);
/* Open the file and truncate it. Trust the symlink. */
if (fd == -1) { if (fd == -1) {
if (!tmp && ISSET(TEMP_OPT)) { if (!tmp && ISSET(TEMP_OPT)) {
UNSET(TEMP_OPT); UNSET(TEMP_OPT);
@ -355,6 +349,25 @@ int write_file(char *name, int tmp)
free(realname); free(realname);
return -1; return -1;
} }
/* Now we fstat() the file, to make sure it's the same file still!
Thanks to Oliver Friedrichs(?) for this code from securityfocus */
if (fstat(fd, &st2) != 0) {
close(fd);
return -1;
}
/* Here we make sure the inode and device numbers are the
* same in the file we actually opened, compared to the file
* we performed the initial lstat() call on.
*/
if (st.st_ino != st2.st_ino || st.st_dev != st2.st_dev) {
close(fd);
return -1;
}
} }
/* Don't follow symlink. Create new file. */ /* Don't follow symlink. Create new file. */
else { else {