Re: [CrystalSpace] #993: Don't read past end of Node->VPath (fix included)

#993: Don't read past end of Node->VPath (fix included)
---------------------------+------------------------------------------------
 Reporter:  ralphcampbell  |       Owner:  jorrit
     Type:  defect         |      Status:  new   
 Priority:  major          |   Milestone:        
Component:  general        |     Version:  V2.1  
 Keywords:                 |  
---------------------------+------------------------------------------------
Description changed by sunshine:

Old description:

> When csVFS::FindFiles() is scanning for files, don't read past the end of
> node->VPath in the memcmp().
> Also, cache the length of VPath for a slight performance increase.
>
> {{{
> Index: plugins/filesys/vfs/vfs.cpp
> ===================================================================
> --- plugins/filesys/vfs/vfs.cpp (revision 39526)
> +++ plugins/filesys/vfs/vfs.cpp (working copy)
> @@ -357,6 +357,7 @@
>  public:
>    // The virtual path
>    char *VPath;
> +  size_t VPath_l;
>    // Configuration section key
>    char *ConfigKey;
>    // The array of real paths/archives bound to this virtual path
> @@ -418,7 +419,7 @@
>    Error = VFS_STATUS_OK;
>    csFile::verbosity = verbosity;
>
> -  size_t vpl = strlen (Node->VPath);
> +  size_t vpl = Node->VPath_l;
>    size_t nsl = strlen (NameSuffix);
>    Name = (char*)cs_malloc (vpl + nsl + 1);
>    memcpy (Name, Node->VPath, vpl);
> @@ -1125,6 +1126,7 @@
>                   csVFS* vfs, unsigned int verbosity) : vfs (vfs)
>  {
>    VPath = iPath;
> +  VPath_l = strlen (iPath);
>    ConfigKey = CS::StrDup (iConfigKey);
>    VfsNode::verbosity = verbosity;
>  }
> @@ -1375,8 +1377,8 @@
>         {
>           vpath << VFS_PATH_SEPARATOR;
>         }
> -    if (FileList->Find (vpath) == csArrayItemNotFound)
> -      FileList->Push (vpath);
> +        if (FileList->Find (vpath) == csArrayItemNotFound)
> +          FileList->Push (vpath);
>        } /* endwhile */
>        closedir (dh);
>      }
> @@ -1414,7 +1416,7 @@
>           }
>           if (cur < fnl)
>             cur++;
> -          size_t vpl = strlen (VPath);
> +          size_t vpl = VPath_l;
>           vpath.Clear();
>           vpath << VPath;
>           vpath << fname;
> @@ -1880,7 +1882,7 @@
>    for (i = 0; i < NodeList.GetSize (); i++)
>    {
>      VfsNode *node = (VfsNode *)NodeList [i];
> -    size_t vpath_l = strlen (node->VPath);
> +    size_t vpath_l = node->VPath_l;
>      if ((vpath_l <= path_l) && (strncmp (node->VPath, Path, vpath_l) ==
> 0))
>      {
>        best_i = i;
> @@ -2062,14 +2064,14 @@
>      for (size_t i = 0; i < NodeList.GetSize (); i++)
>      {
>        VfsNode *node = (VfsNode *)NodeList [i];
> -      if ((memcmp (node->VPath, XPath, sl) == 0) && (node->VPath [sl]))
> +      if (node->VPath_l > sl && (memcmp (node->VPath, XPath, sl) == 0)
> && (node->VPath [sl]))
>        {
>          const char *pp = node->VPath + sl;
> -        while (*pp && *pp == VFS_PATH_SEPARATOR)
> +        while (*pp == VFS_PATH_SEPARATOR)
>            pp++;
>          while (*pp && *pp != VFS_PATH_SEPARATOR)
>            pp++;
> -        while (*pp && *pp == VFS_PATH_SEPARATOR)
> +        while (*pp == VFS_PATH_SEPARATOR)
>            pp++;
>          news.Clear();
>          news.Append (node->VPath);
> }}}

New description:

 When `csVFS::FindFiles()` is scanning for files, don't read past the end
 of `node->VPath` in the `memcmp()`.
 Also, cache the length of `VPath` for a slight performance increase.

 {{{
 #!diff
 Index: plugins/filesys/vfs/vfs.cpp
 ===================================================================
 --- plugins/filesys/vfs/vfs.cpp (revision 39526)
 +++ plugins/filesys/vfs/vfs.cpp (working copy)
 @@ -357,6 +357,7 @@
  public:
    // The virtual path
    char *VPath;
 +  size_t VPath_l;
    // Configuration section key
    char *ConfigKey;
    // The array of real paths/archives bound to this virtual path
 @@ -418,7 +419,7 @@
    Error = VFS_STATUS_OK;
    csFile::verbosity = verbosity;

 -  size_t vpl = strlen (Node->VPath);
 +  size_t vpl = Node->VPath_l;
    size_t nsl = strlen (NameSuffix);
    Name = (char*)cs_malloc (vpl + nsl + 1);
    memcpy (Name, Node->VPath, vpl);
 @@ -1125,6 +1126,7 @@
                   csVFS* vfs, unsigned int verbosity) : vfs (vfs)
  {
    VPath = iPath;
 +  VPath_l = strlen (iPath);
    ConfigKey = CS::StrDup (iConfigKey);
    VfsNode::verbosity = verbosity;
  }
 @@ -1375,8 +1377,8 @@
         {
           vpath << VFS_PATH_SEPARATOR;
         }
 -    if (FileList->Find (vpath) == csArrayItemNotFound)
 -      FileList->Push (vpath);
 +        if (FileList->Find (vpath) == csArrayItemNotFound)
 +          FileList->Push (vpath);
        } /* endwhile */
        closedir (dh);
      }
 @@ -1414,7 +1416,7 @@
           }
           if (cur < fnl)
             cur++;
 -          size_t vpl = strlen (VPath);
 +          size_t vpl = VPath_l;
           vpath.Clear();
           vpath << VPath;
           vpath << fname;
 @@ -1880,7 +1882,7 @@
    for (i = 0; i < NodeList.GetSize (); i++)
    {
      VfsNode *node = (VfsNode *)NodeList [i];
 -    size_t vpath_l = strlen (node->VPath);
 +    size_t vpath_l = node->VPath_l;
      if ((vpath_l <= path_l) && (strncmp (node->VPath, Path, vpath_l) ==
 0))
      {
        best_i = i;
 @@ -2062,14 +2064,14 @@
      for (size_t i = 0; i < NodeList.GetSize (); i++)
      {
        VfsNode *node = (VfsNode *)NodeList [i];
 -      if ((memcmp (node->VPath, XPath, sl) == 0) && (node->VPath [sl]))
 +      if (node->VPath_l > sl && (memcmp (node->VPath, XPath, sl) == 0) &&
 (node->VPath [sl]))
        {
          const char *pp = node->VPath + sl;
 -        while (*pp && *pp == VFS_PATH_SEPARATOR)
 +        while (*pp == VFS_PATH_SEPARATOR)
            pp++;
          while (*pp && *pp != VFS_PATH_SEPARATOR)
            pp++;
 -        while (*pp && *pp == VFS_PATH_SEPARATOR)
 +        while (*pp == VFS_PATH_SEPARATOR)
            pp++;
          news.Clear();
          news.Append (node->VPath);
 }}}

--

-- 
Ticket URL: <http://www.crystalspace3d.org/trac/CS/ticket/993#comment:1>
CrystalSpace <http://www.crystalspace3d.org/>
Free open-source 3D SDK.
------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent 
caught up. So what steps can you take to put your SQL databases under 
version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk