Don't allow NULL filenames to be mounted.
authorRyan C. Gordon <icculus@icculus.org>
Mon, 23 Oct 2017 12:40:59 -0400
changeset 1618 0bbfaf6c5508
parent 1617 67ca4c4f043e
child 1619 4f1bf89597e5
Don't allow NULL filenames to be mounted.

Regardless of what the 3.0.0 documentation says, PhysicsFS never handled this
correctly, so now we check for it so you can't get into crashy situations.

Corrected documentation to reflect reality.
src/physfs.c
src/physfs.h
--- a/src/physfs.c	Mon Oct 23 12:16:51 2017 -0400
+++ b/src/physfs.c	Mon Oct 23 12:40:59 2017 -0400
@@ -1012,6 +1012,8 @@
     DirHandle *dirHandle = NULL;
     char *tmpmntpnt = NULL;
 
+    assert(newDir != NULL);  /* should have caught this higher up. */
+
     if (mountPoint != NULL)
     {
         const size_t len = strlen(mountPoint) + 1;
@@ -1025,15 +1027,9 @@
     dirHandle = openDirectory(io, newDir, forWriting);
     GOTO_IF_ERRPASS(!dirHandle, badDirHandle);
 
-    if (newDir == NULL)
-        dirHandle->dirName = NULL;
-    else
-    {
-        dirHandle->dirName = (char *) allocator.Malloc(strlen(newDir) + 1);
-        if (!dirHandle->dirName)
-            GOTO(PHYSFS_ERR_OUT_OF_MEMORY, badDirHandle);
-        strcpy(dirHandle->dirName, newDir);
-    } /* else */
+    dirHandle->dirName = (char *) allocator.Malloc(strlen(newDir) + 1);
+    GOTO_IF(!dirHandle->dirName, PHYSFS_ERR_OUT_OF_MEMORY, badDirHandle);
+    strcpy(dirHandle->dirName, newDir);
 
     if ((mountPoint != NULL) && (*mountPoint != '\0'))
     {
@@ -1684,21 +1680,20 @@
     DirHandle *prev = NULL;
     DirHandle *i;
 
+    BAIL_IF(!fname, PHYSFS_ERR_INVALID_ARGUMENT, 0);
+
     if (mountPoint == NULL)
         mountPoint = "/";
 
     __PHYSFS_platformGrabMutex(stateLock);
 
-    if (fname != NULL)
+    for (i = searchPath; i != NULL; i = i->next)
     {
-        for (i = searchPath; i != NULL; i = i->next)
-        {
-            /* already in search path? */
-            if ((i->dirName != NULL) && (strcmp(fname, i->dirName) == 0))
-                BAIL_MUTEX_ERRPASS(stateLock, 1);
-            prev = i;
-        } /* for */
-    } /* if */
+        /* already in search path? */
+        if ((i->dirName != NULL) && (strcmp(fname, i->dirName) == 0))
+            BAIL_MUTEX_ERRPASS(stateLock, 1);
+        prev = i;
+    } /* for */
 
     dh = createDirHandle(io, fname, mountPoint, 0);
     BAIL_IF_MUTEX_ERRPASS(!dh, stateLock, 0);
@@ -1725,6 +1720,7 @@
                    const char *mountPoint, int appendToPath)
 {
     BAIL_IF(!io, PHYSFS_ERR_INVALID_ARGUMENT, 0);
+    BAIL_IF(!fname, PHYSFS_ERR_INVALID_ARGUMENT, 0);
     BAIL_IF(io->version != 0, PHYSFS_ERR_UNSUPPORTED, 0);
     return doMount(io, fname, mountPoint, appendToPath);
 } /* PHYSFS_mountIo */
@@ -1738,6 +1734,7 @@
     PHYSFS_Io *io = NULL;
 
     BAIL_IF(!buf, PHYSFS_ERR_INVALID_ARGUMENT, 0);
+    BAIL_IF(!fname, PHYSFS_ERR_INVALID_ARGUMENT, 0);
 
     io = __PHYSFS_createMemoryIo(buf, len, del);
     BAIL_IF_ERRPASS(!io, 0);
@@ -1760,7 +1757,8 @@
     int retval = 0;
     PHYSFS_Io *io = NULL;
 
-    BAIL_IF(file == NULL, PHYSFS_ERR_INVALID_ARGUMENT, 0);
+    BAIL_IF(!file, PHYSFS_ERR_INVALID_ARGUMENT, 0);
+    BAIL_IF(!fname, PHYSFS_ERR_INVALID_ARGUMENT, 0);
 
     io = __PHYSFS_createHandleIo(file);
     BAIL_IF_ERRPASS(!io, 0);
@@ -1785,7 +1783,7 @@
 
 int PHYSFS_addToSearchPath(const char *newDir, int appendToPath)
 {
-    return doMount(NULL, newDir, NULL, appendToPath);
+    return PHYSFS_mount(newDir, NULL, appendToPath);
 } /* PHYSFS_addToSearchPath */
 
 
--- a/src/physfs.h	Mon Oct 23 12:16:51 2017 -0400
+++ b/src/physfs.h	Mon Oct 23 12:40:59 2017 -0400
@@ -2760,6 +2760,12 @@
  * This call will fail (and fail to remove from the path) if the element still
  *  has files open in it.
  *
+ * \warning This function wants the path to the archive or directory that was
+ *          mounted (the same string used for the "newDir" argument of
+ *          PHYSFS_addToSearchPath or any of the mount functions), not the
+ *          path where it is mounted in the tree (the "mountPoint" argument
+ *          to any of the mount functions).
+ *
  *    \param oldDir dir/archive to remove.
  *   \return nonzero on success, zero on failure. Use
  *           PHYSFS_getLastErrorCode() to obtain the specific error.
@@ -3188,7 +3194,7 @@
 
 
 /**
- * \fn int PHYSFS_mountIo(PHYSFS_Io *io, const char *fname, const char *mountPoint, int appendToPath)
+ * \fn int PHYSFS_mountIo(PHYSFS_Io *io, const char *newDir, const char *mountPoint, int appendToPath)
  * \brief Add an archive, built on a PHYSFS_Io, to the search path.
  *
  * \warning Unless you have some special, low-level need, you should be using
@@ -3198,11 +3204,14 @@
  *  instead of a pathname. Behind the scenes, PHYSFS_mount() calls this
  *  function with a physical-filesystem-based PHYSFS_Io.
  *
- * (filename) is only used here to optimize archiver selection (if you name it
- *  XXXXX.zip, we might try the ZIP archiver first, for example). It doesn't
- *  need to refer to a real file at all, and can even be NULL. If the filename
- *  isn't helpful, the system will try every archiver until one works or none
- *  of them do.
+ * (newDir) must be a unique string to identify this archive. It is used
+ *  to optimize archiver selection (if you name it XXXXX.zip, we might try
+ *  the ZIP archiver first, for example, or directly choose an archiver that
+ *  can only trust the data is valid by filename extension). It doesn't
+ *  need to refer to a real file at all. If the filename extension isn't
+ *  helpful, the system will try every archiver until one works or none
+ *  of them do. This filename must be unique, as the system won't allow you
+ *  to have two archives with the same name.
  *
  * (io) must remain until the archive is unmounted. When the archive is
  *  unmounted, the system will call (io)->destroy(io), which will give you
@@ -3211,7 +3220,7 @@
  * If this function fails, (io)->destroy(io) is not called.
  *
  *   \param io i/o instance for archive to add to the path.
- *   \param fname Filename that can represent this stream. Can be NULL.
+ *   \param newDir Filename that can represent this stream.
  *   \param mountPoint Location in the interpolated tree that this archive
  *                     will be "mounted", in platform-independent notation.
  *                     NULL or "" is equivalent to "/".
@@ -3224,12 +3233,12 @@
  * \sa PHYSFS_getSearchPath
  * \sa PHYSFS_getMountPoint
  */
-PHYSFS_DECL int PHYSFS_mountIo(PHYSFS_Io *io, const char *fname,
+PHYSFS_DECL int PHYSFS_mountIo(PHYSFS_Io *io, const char *newDir,
                                const char *mountPoint, int appendToPath);
 
 
 /**
- * \fn int PHYSFS_mountMemory(const void *buf, PHYSFS_uint64 len, void (*del)(void *), const char *fname, const char *mountPoint, int appendToPath)
+ * \fn int PHYSFS_mountMemory(const void *buf, PHYSFS_uint64 len, void (*del)(void *), const char *newDir, const char *mountPoint, int appendToPath)
  * \brief Add an archive, contained in a memory buffer, to the search path.
  *
  * \warning Unless you have some special, low-level need, you should be using
@@ -3239,11 +3248,14 @@
  *  instead of a pathname. This buffer contains all the data of the archive,
  *  and is used instead of a real file in the physical filesystem.
  *
- * (filename) is only used here to optimize archiver selection (if you name it
- *  XXXXX.zip, we might try the ZIP archiver first, for example). It doesn't
- *  need to refer to a real file at all, and can even be NULL. If the filename
- *  isn't helpful, the system will try every archiver until one works or none
- *  of them do.
+ * (newDir) must be a unique string to identify this archive. It is used
+ *  to optimize archiver selection (if you name it XXXXX.zip, we might try
+ *  the ZIP archiver first, for example, or directly choose an archiver that
+ *  can only trust the data is valid by filename extension). It doesn't
+ *  need to refer to a real file at all. If the filename extension isn't
+ *  helpful, the system will try every archiver until one works or none
+ *  of them do. This filename must be unique, as the system won't allow you
+ *  to have two archives with the same name.
  *
  * (ptr) must remain until the archive is unmounted. When the archive is
  *  unmounted, the system will call (del)(ptr), which will notify you that
@@ -3256,7 +3268,7 @@
  *   \param buf Address of the memory buffer containing the archive data.
  *   \param len Size of memory buffer, in bytes.
  *   \param del A callback that triggers upon unmount. Can be NULL.
- *   \param fname Filename that can represent this stream. Can be NULL.
+ *   \param newDir Filename that can represent this stream.
  *   \param mountPoint Location in the interpolated tree that this archive
  *                     will be "mounted", in platform-independent notation.
  *                     NULL or "" is equivalent to "/".
@@ -3269,12 +3281,12 @@
  * \sa PHYSFS_getMountPoint
  */
 PHYSFS_DECL int PHYSFS_mountMemory(const void *buf, PHYSFS_uint64 len,
-                                   void (*del)(void *), const char *fname,
+                                   void (*del)(void *), const char *newDir,
                                    const char *mountPoint, int appendToPath);
 
 
 /**
- * \fn int PHYSFS_mountHandle(PHYSFS_File *file, const char *fname, const char *mountPoint, int appendToPath)
+ * \fn int PHYSFS_mountHandle(PHYSFS_File *file, const char *newDir, const char *mountPoint, int appendToPath)
  * \brief Add an archive, contained in a PHYSFS_File handle, to the search path.
  *
  * \warning Unless you have some special, low-level need, you should be using
@@ -3297,11 +3309,14 @@
  *  but isn't necessarily. The most popular use for this is likely to mount
  *  archives stored inside other archives.
  *
- * (filename) is only used here to optimize archiver selection (if you name it
- *  XXXXX.zip, we might try the ZIP archiver first, for example). It doesn't
- *  need to refer to a real file at all, and can even be NULL. If the filename
- *  isn't helpful, the system will try every archiver until one works or none
- *  of them do.
+ * (newDir) must be a unique string to identify this archive. It is used
+ *  to optimize archiver selection (if you name it XXXXX.zip, we might try
+ *  the ZIP archiver first, for example, or directly choose an archiver that
+ *  can only trust the data is valid by filename extension). It doesn't
+ *  need to refer to a real file at all. If the filename extension isn't
+ *  helpful, the system will try every archiver until one works or none
+ *  of them do. This filename must be unique, as the system won't allow you
+ *  to have two archives with the same name.
  *
  * (file) must remain until the archive is unmounted. When the archive is
  *  unmounted, the system will call PHYSFS_close(file). If you need this
@@ -3311,7 +3326,7 @@
  * If this function fails, PHYSFS_close(file) is not called.
  *
  *   \param file The PHYSFS_File handle containing archive data.
- *   \param fname Filename that can represent this stream. Can be NULL.
+ *   \param newDir Filename that can represent this stream.
  *   \param mountPoint Location in the interpolated tree that this archive
  *                     will be "mounted", in platform-independent notation.
  *                     NULL or "" is equivalent to "/".
@@ -3323,7 +3338,7 @@
  * \sa PHYSFS_getSearchPath
  * \sa PHYSFS_getMountPoint
  */
-PHYSFS_DECL int PHYSFS_mountHandle(PHYSFS_File *file, const char *fname,
+PHYSFS_DECL int PHYSFS_mountHandle(PHYSFS_File *file, const char *newDir,
                                    const char *mountPoint, int appendToPath);