summaryrefslogtreecommitdiff
authorerik <erik>2007-01-26 20:30:32 (UTC)
committer erik <erik>2007-01-26 20:30:32 (UTC)
commitf77da1ae08512b02a3c50a124f823ed77e53dd64 (patch) (unidiff)
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) (ignore whitespace changes)
-rw-r--r--core/apps/textedit/textedit.cpp45
-rw-r--r--core/launcher/packageslave.cpp48
2 files changed, 49 insertions, 44 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
@@ -781,5 +781,5 @@ void TextEdit::showEditTools() {
781 unprompted save */ 781 unprompted save */
782bool TextEdit::save() { 782bool TextEdit::save() {
783 QString name, file; 783 QString name, file;
784 odebug << "saveAsFile " + currentFileName << oendl; 784 odebug << "saveAsFile " + currentFileName << oendl;
785 if(currentFileName.isEmpty()) { 785 if(currentFileName.isEmpty()) {
@@ -787,14 +787,13 @@ bool TextEdit::save() {
787 return false; 787 return false;
788 } 788 }
789 name = currentFileName; 789 if(doc) {
790 if(doc) { 790 file = doc->file();
791 file = doc->file(); 791 odebug << "saver file "+file << oendl;
792 odebug << "saver file "+file << oendl; 792 name = doc->name();
793 name = doc->name(); 793 odebug << "File named "+name << oendl;
794 odebug << "File named "+name << oendl; 794 } else {
795 } else { 795 file = currentFileName;
796 file = currentFileName;
797 name = QFileInfo(currentFileName).baseName(); 796 name = QFileInfo(currentFileName).baseName();
798 } 797 }
799 798
800 QString rt = editor->text(); 799 QString rt = editor->text();
@@ -808,5 +807,6 @@ bool TextEdit::save() {
808 struct stat buf; 807 struct stat buf;
809 mode_t mode; 808 mode_t mode;
810 stat(file.latin1(), &buf); 809 QFile f(file);
810 fstat(f.handle(), &buf);
811 mode = buf.st_mode; 811 mode = buf.st_mode;
812 812
@@ -815,18 +815,16 @@ bool TextEdit::save() {
815 FileManager fm; 815 FileManager fm;
816 if ( !fm.saveFile( *doc, rt ) ) { 816 if ( !fm.saveFile( *doc, rt ) ) {
817 QMessageBox::message(tr("Text Edit"),tr("Save Failed")); 817 QMessageBox::message(tr("Text Edit"),tr("Save Failed"));
818 return false; 818 return false;
819 } 819 }
820 } else { 820 } else {
821 odebug << "regular save file" << oendl; 821 odebug << "regular save file" << oendl;
822 QFile f(file); 822 if( f.open(IO_WriteOnly)) {
823 if( f.open(IO_WriteOnly)) { 823 QCString crt = rt.utf8();
824 QCString crt = rt.utf8(); 824 f.writeBlock(crt,crt.length());
825 f.writeBlock(crt,crt.length()); 825 } else {
826 } else { 826 QMessageBox::message(tr("Text Edit"),tr("Write Failed"));
827 QMessageBox::message(tr("Text Edit"),tr("Write Failed")); 827 return false;
828 return false; 828 }
829 }
830
831 } 829 }
832 editor->setEdited( false); 830 editor->setEdited( false);
@@ -834,8 +832,7 @@ bool TextEdit::save() {
834 edited=false; 832 edited=false;
835 if(caption().left(1)=="*") 833 if(caption().left(1)=="*")
836 setCaption(caption().right(caption().length()-1)); 834 setCaption(caption().right(caption().length()-1));
837
838 835
839 chmod( file.latin1(), mode); 836 fchmod( f.handle(), mode);
840 } 837 }
841 return true; 838 return true;
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
@@ -215,30 +215,38 @@ void PackageHandler::cleanupPackageFiles( const QString &listfile )
215 215
216 if ( f.open(IO_ReadOnly) ) { 216 if ( f.open(IO_ReadOnly) ) {
217 QTextStream ts(&f); 217 QTextStream ts(&f);
218 218
219 QString s; 219 QString s;
220 while ( !ts.eof() ) { // until end of file... 220 while ( !ts.eof() ) { // until end of file...
221 s = ts.readLine(); // line of text excluding '\n' 221 s = ts.readLine(); // line of text excluding '\n'
222 // for s, do link/mkdir. 222 // for s, do link/mkdir.
223 if ( s.right(1) == "/" ) { 223 // @todo Right now we just move on if the name of the file we
224 //should rmdir if empty, after all files have been removed 224 // find is actually a directory. What we ought to do is check
225 } else { 225 // to see if the directory is empty and if so remove it.
226 if ( s.right(1) != "/" ) {
226#ifndef Q_OS_WIN32 227#ifndef Q_OS_WIN32
227 odebug << "remove symlink for " << s.ascii() << "" << oendl; 228 odebug << "remove symlink for " << s << oendl;
228 //check if it is a symlink first (don't remove /etc/passwd...) 229 QFile symFile(s);
229 char buf[10]; //we don't care about the contents 230 QFileInfo symFileInfo(symFile);
230 if ( ::readlink( s.ascii(),buf, 10 >= 0 ) ) 231 //check if it is a symlink first (don't remove /etc/passwd...)
231 ::unlink( s.ascii() ); 232 if ( !symFileInfo.readLink().isNull())
233 if (!symFile.remove())
234 owarn << "Unable to remove symlink " << symFile.name()
235 << " " << __FILE__ << ":" << __LINE__ << oendl;
232#else 236#else
233 // ### revise 237 // @todo If we actually want to be portable to other operating
234 owarn << "Unable to remove symlink " << __FILE__ << ":" << __LINE__ << "" << oendl; 238 // systems we ought to at least have a portable way of removing
239 // their notion of symlinks.
240 owarn << "Unable to remove symlink " << s " " << __FILE__
241 << ":" << __LINE__ << oendl;
235#endif 242#endif
243 }
236 } 244 }
237 } 245 f.close();
238 f.close();
239 246
240 //remove the list file 247 //remove the list file
241 ::unlink( listfile.ascii() ); 248 if (!f.remove())
242 249 owarn << "Unable to remove list file " << f.name() << " "
250 << __FILE__ << ":" << __LINE__ << oendl;
243 } 251 }
244} 252}