From: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
Date: Wed, 7 Jan 2015 08:34:07 +0000 (+0100)
Subject: osd/ClassHandler.cc: move stat into error handling
X-Git-Tag: v0.92~23^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b99508b30cd53716e15d5464136a841c9a66a56f;p=ceph.git

osd/ClassHandler.cc: move stat into error handling

There is no security advantage to check if the class file
exists before opening it but the file could be removed or
exchanged between the stat and open. Instead directly open
it and fail. Check if the file was missing afterwards
for debug messages and error codes.

Make sure cls->status is set if the class open call fails.

To solve Coverity issue:

CID 743419 (#1 of 1): Time of check time of use (TOCTOU)
 fs_check_call: Calling function stat to perform check on fname.

743419 Time of check time of use
 An attacker could change the filename's file association or
 other attributes between the check and use.

 In ClassHandler::_load_class(ClassHandler::ClassData *): A check
 occurs on a file's attributes before the file is used in a
 privileged operation, but things may have changed (CWE-367)

Signed-off-by: Danny Al-Gaaf <danny.al-gaaf@bisect.de>
---

diff --git a/src/osd/ClassHandler.cc b/src/osd/ClassHandler.cc
index 188f7210d8af..6c1f20dbaa3a 100644
--- a/src/osd/ClassHandler.cc
+++ b/src/osd/ClassHandler.cc
@@ -1,3 +1,5 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- 
+// vim: ts=8 sw=2 smarttab
 
 #include "include/types.h"
 #include "msg/Message.h"
@@ -106,21 +108,21 @@ int ClassHandler::_load_class(ClassData *cls)
 	     cls->name.c_str());
     dout(10) << "_load_class " << cls->name << " from " << fname << dendl;
 
-    struct stat st;
-    int r = ::stat(fname, &st);
-    if (r < 0) {
-      r = -errno;
-      dout(0) << __func__ << " could not stat class " << fname
-	      << ": " << cpp_strerror(r) << dendl;
-      return r;
-    }
-
     cls->handle = dlopen(fname, RTLD_NOW);
     if (!cls->handle) {
-      dout(0) << "_load_class could not open class " << fname
-	      << " (dlopen failed): " << dlerror() << dendl;
+      struct stat st;
+      int r = ::stat(fname, &st);
+      if (r < 0) {
+        r = -errno;
+        dout(0) << __func__ << " could not stat class " << fname
+                << ": " << cpp_strerror(r) << dendl;
+      } else {
+	dout(0) << "_load_class could not open class " << fname
+      	        << " (dlopen failed): " << dlerror() << dendl;
+      	r = -EIO;
+      }
       cls->status = ClassData::CLASS_MISSING;
-      return -EIO;
+      return r;
     }
 
     cls_deps_t *(*cls_deps)();