summaryrefslogtreecommitdiff
authorerik <erik>2007-01-26 20:30:32 (UTC)
committer erik <erik>2007-01-26 20:30:32 (UTC)
commitf77da1ae08512b02a3c50a124f823ed77e53dd64 (patch) (side-by-side diff)
treeac7e414aff95348e0bf2fba3f45b2a06a4eb4623
parent4688f98202f590ec6af6c2e66a49dd2f80536083 (diff)
downloadopie-f77da1ae08512b02a3c50a124f823ed77e53dd64.zip
opie-f77da1ae08512b02a3c50a124f823ed77e53dd64.tar.gz
opie-f77da1ae08512b02a3c50a124f823ed77e53dd64.tar.bz2
Both packageslave.cpp and textedit.cpp have instances of possibly exploitable
race conditions associated to files. The big deal is that it is quite typical to use strings of pathnames to track files. But because that does not leverage the filesystem would be attackers may be able to exploit time lags in uses of filesystem functions (like stat and chmod or open) to get files with suspect data into the files that the applications are working with. This commit closes that potential hole even though there are no known exploits. Better safe then sorry. There is no change in the behavior of the apps.
Diffstat (more/less context) (show whitespace changes)
-rw-r--r--core/apps/textedit/textedit.cpp9
-rw-r--r--core/launcher/packageslave.cpp30
2 files changed, 22 insertions, 17 deletions
diff --git a/core/apps/textedit/textedit.cpp b/core/apps/textedit/textedit.cpp
index 4bbc62b..1c81a55 100644
--- a/core/apps/textedit/textedit.cpp
+++ b/core/apps/textedit/textedit.cpp
@@ -783,13 +783,12 @@ bool TextEdit::save() {
QString name, file;
odebug << "saveAsFile " + currentFileName << oendl;
if(currentFileName.isEmpty()) {
saveAs();
return false;
}
- name = currentFileName;
if(doc) {
file = doc->file();
odebug << "saver file "+file << oendl;
name = doc->name();
odebug << "File named "+name << oendl;
} else {
@@ -804,42 +803,40 @@ bool TextEdit::save() {
} else {
currentFileName = name;
odebug << "saveFile "+currentFileName << oendl;
struct stat buf;
mode_t mode;
- stat(file.latin1(), &buf);
+ QFile f(file);
+ fstat(f.handle(), &buf);
mode = buf.st_mode;
if(!fileIs) {
doc->setName( name);
FileManager fm;
if ( !fm.saveFile( *doc, rt ) ) {
QMessageBox::message(tr("Text Edit"),tr("Save Failed"));
return false;
}
} else {
odebug << "regular save file" << oendl;
- QFile f(file);
if( f.open(IO_WriteOnly)) {
QCString crt = rt.utf8();
f.writeBlock(crt,crt.length());
} else {
QMessageBox::message(tr("Text Edit"),tr("Write Failed"));
return false;
}
-
}
editor->setEdited( false);
edited1=false;
edited=false;
if(caption().left(1)=="*")
setCaption(caption().right(caption().length()-1));
-
- chmod( file.latin1(), mode);
+ fchmod( f.handle(), mode);
}
return true;
}
return false;
}
diff --git a/core/launcher/packageslave.cpp b/core/launcher/packageslave.cpp
index abbc610..965020e 100644
--- a/core/launcher/packageslave.cpp
+++ b/core/launcher/packageslave.cpp
@@ -217,32 +217,40 @@ void PackageHandler::cleanupPackageFiles( const QString &listfile )
QTextStream ts(&f);
QString s;
while ( !ts.eof() ) { // until end of file...
s = ts.readLine(); // line of text excluding '\n'
// for s, do link/mkdir.
- if ( s.right(1) == "/" ) {
- //should rmdir if empty, after all files have been removed
- } else {
+ // @todo Right now we just move on if the name of the file we
+ // find is actually a directory. What we ought to do is check
+ // to see if the directory is empty and if so remove it.
+ if ( s.right(1) != "/" ) {
#ifndef Q_OS_WIN32
- odebug << "remove symlink for " << s.ascii() << "" << oendl;
+ odebug << "remove symlink for " << s << oendl;
+ QFile symFile(s);
+ QFileInfo symFileInfo(symFile);
//check if it is a symlink first (don't remove /etc/passwd...)
- char buf[10]; //we don't care about the contents
- if ( ::readlink( s.ascii(),buf, 10 >= 0 ) )
- ::unlink( s.ascii() );
+ if ( !symFileInfo.readLink().isNull())
+ if (!symFile.remove())
+ owarn << "Unable to remove symlink " << symFile.name()
+ << " " << __FILE__ << ":" << __LINE__ << oendl;
#else
- // ### revise
- owarn << "Unable to remove symlink " << __FILE__ << ":" << __LINE__ << "" << oendl;
+ // @todo If we actually want to be portable to other operating
+ // systems we ought to at least have a portable way of removing
+ // their notion of symlinks.
+ owarn << "Unable to remove symlink " << s " " << __FILE__
+ << ":" << __LINE__ << oendl;
#endif
}
}
f.close();
//remove the list file
- ::unlink( listfile.ascii() );
-
+ if (!f.remove())
+ owarn << "Unable to remove list file " << f.name() << " "
+ << __FILE__ << ":" << __LINE__ << oendl;
}
}
void PackageHandler::cleanupPackages( const QString &location )
{
// get list of *.list in location/usr/lib/ipkg/info/*.list