author | erik <erik> | 2007-01-26 20:30:32 (UTC) |
---|---|---|
committer | erik <erik> | 2007-01-26 20:30:32 (UTC) |
commit | f77da1ae08512b02a3c50a124f823ed77e53dd64 (patch) (unidiff) | |
tree | ac7e414aff95348e0bf2fba3f45b2a06a4eb4623 /core/launcher | |
parent | 4688f98202f590ec6af6c2e66a49dd2f80536083 (diff) | |
download | opie-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.
-rw-r--r-- | core/launcher/packageslave.cpp | 48 |
1 files changed, 28 insertions, 20 deletions
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 | |||
@@ -193,74 +193,82 @@ void PackageHandler::addPackageFiles( const QString &location, | |||
193 | } | 193 | } |
194 | } | 194 | } |
195 | 195 | ||
196 | void PackageHandler::addPackages( const QString &location ) | 196 | void PackageHandler::addPackages( const QString &location ) |
197 | { | 197 | { |
198 | // get list of *.list in location/usr/lib/ipkg/info/*.list | 198 | // get list of *.list in location/usr/lib/ipkg/info/*.list |
199 | QDir dir(location + "/usr/lib/ipkg/info", "*.list", // No tr | 199 | QDir dir(location + "/usr/lib/ipkg/info", "*.list", // No tr |
200 | QDir::Name, QDir::Files); | 200 | QDir::Name, QDir::Files); |
201 | if ( !dir.exists() ) | 201 | if ( !dir.exists() ) |
202 | return; | 202 | return; |
203 | 203 | ||
204 | QStringList packages = dir.entryList(); | 204 | QStringList packages = dir.entryList(); |
205 | for ( QStringList::Iterator it = packages.begin(); | 205 | for ( QStringList::Iterator it = packages.begin(); |
206 | it != packages.end(); ++it ) { | 206 | it != packages.end(); ++it ) { |
207 | addPackageFiles( location, *it ); | 207 | addPackageFiles( location, *it ); |
208 | } | 208 | } |
209 | } | 209 | } |
210 | 210 | ||
211 | 211 | ||
212 | void PackageHandler::cleanupPackageFiles( const QString &listfile ) | 212 | void PackageHandler::cleanupPackageFiles( const QString &listfile ) |
213 | { | 213 | { |
214 | QFile f(listfile); | 214 | QFile f(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 | } |
245 | 253 | ||
246 | void PackageHandler::cleanupPackages( const QString &location ) | 254 | void PackageHandler::cleanupPackages( const QString &location ) |
247 | { | 255 | { |
248 | // get list of *.list in location/usr/lib/ipkg/info/*.list | 256 | // get list of *.list in location/usr/lib/ipkg/info/*.list |
249 | QDir dir( "/usr/lib/ipkg/info/"+location, "*.list", // No tr | 257 | QDir dir( "/usr/lib/ipkg/info/"+location, "*.list", // No tr |
250 | QDir::Name, QDir::Files); | 258 | QDir::Name, QDir::Files); |
251 | if ( !dir.exists() ) | 259 | if ( !dir.exists() ) |
252 | return; | 260 | return; |
253 | 261 | ||
254 | QStringList packages = dir.entryList(); | 262 | QStringList packages = dir.entryList(); |
255 | for ( QStringList::Iterator it = packages.begin(); | 263 | for ( QStringList::Iterator it = packages.begin(); |
256 | it != packages.end(); ++it ) { | 264 | it != packages.end(); ++it ) { |
257 | cleanupPackageFiles( *it ); | 265 | cleanupPackageFiles( *it ); |
258 | } | 266 | } |
259 | 267 | ||
260 | //remove the backup directory | 268 | //remove the backup directory |
261 | //### | 269 | //### |
262 | } | 270 | } |
263 | 271 | ||
264 | void PackageHandler::prepareInstall( const QString& size, const QString& path ) | 272 | void PackageHandler::prepareInstall( const QString& size, const QString& path ) |
265 | { | 273 | { |
266 | // Check whether there will be enough space to install the next package. | 274 | // Check whether there will be enough space to install the next package. |