Define one constant, PLATFORM_MAX_PATH, for maximum path value, so we don't have to do platform-specific conditionals everywhere we want to check it. In fact, we were actually checking that only for Windows. This is follow-on to Richard's message on audacity-devel about "overflow vulns".

Remove unnecessary declaration of MAX_PATH in BlockFile.h.

src/Audacity.h

@@ -1,11 +1,14 @@
 /**********************************************************************
 
-  Audacity: A Digital Audio Editor
+   Audacity: A Digital Audio Editor
+   Audacity(R) is copyright (c) 1999-2011 Audacity Team.
+   License: GPL v2.  See License.txt.
 
-  Audacity.h
+   Audacity.h
 
-  Dominic Mazzoni
+   Dominic Mazzoni
-  Joshua Haberman
+   Joshua Haberman
+   et al
 
 ********************************************************************//*!
 
@@ -69,20 +72,31 @@ class wxWindow;
 void QuitAudacity(bool bForce);
 void QuitAudacity();
 
+// Define one constant for maximum path value, so we don't have to do 
+// platform-specific conditionals everywhere we want to check it. 
+#define PLATFORM_MAX_PATH 260 // Play it safe for default, with same value as Windows' MAX_PATH.
+
 #ifdef __WXMAC__
 #include "configmac.h"
+#undef PLATFORM_MAX_PATH
+#define PLATFORM_MAX_PATH PATH_MAX
 #endif
 
 #ifdef __WXGTK__
 #include "configunix.h"
+#undef PLATFORM_MAX_PATH
+#define PLATFORM_MAX_PATH PATH_MAX
 #endif
 
 #ifdef __WXX11__
 #include "configunix.h"
+// wxX11 should also get the platform-specific definition of PLATFORM_MAX_PATH, so do not declare here.
 #endif
 
 #ifdef __WXMSW__
 #include "configwin.h"
+#undef PLATFORM_MAX_PATH
+#define PLATFORM_MAX_PATH MAX_PATH
 #endif
 
 /* Magic for dynamic library import and export. This is unfortunately

src/BlockFile.h

@@ -21,14 +21,6 @@
 #include "xml/XMLTagHandler.h"
 #include "xml/XMLWriter.h"
 
-#if defined(_WIN32)
-  //taken from private.h (wxWidgets internal declarations)
-  #ifndef MAX_PATH
-    #define MAX_PATH  260
-  #endif
-#endif
-
-class wxFFile;
 
 class SummaryInfo {
  public:

src/blockfile/ODDecodeBlockFile.cpp

@@ -238,11 +238,8 @@ BlockFile *ODDecodeBlockFile::BuildFromXML(DirManager &dm, const wxChar **attrs)
       const wxString strValue = value;
       if (!wxStricmp(attr, wxT("summaryfile")) && 
             // Can't use XMLValueChecker::IsGoodFileName here, but do part of its test.
-            XMLValueChecker::IsGoodFileString(strValue)
+            XMLValueChecker::IsGoodFileString(strValue) && 
-            #ifdef _WIN32
+            (strValue.Length() + 1 + dm.GetProjectDataDir().Length() <= PLATFORM_MAX_PATH))
-               && (strValue.Length() + 1 + dm.GetProjectDataDir().Length() <= MAX_PATH)
-            #endif
-            )
       {
          if (!dm.AssignFile(summaryFileName, strValue, false))
             // Make sure summaryFileName is back to uninitialized state so we can detect problem later.

src/blockfile/ODPCMAliasBlockFile.cpp

@@ -305,11 +305,8 @@ BlockFile *ODPCMAliasBlockFile::BuildFromXML(DirManager &dm, const wxChar **attr
       const wxString strValue = value;
       if (!wxStricmp(attr, wxT("summaryfile")) && 
             // Can't use XMLValueChecker::IsGoodFileName here, but do part of its test.
-            XMLValueChecker::IsGoodFileString(strValue)
+            XMLValueChecker::IsGoodFileString(strValue) && 
-            #ifdef _WIN32
+            (strValue.Length() + 1 + dm.GetProjectDataDir().Length() <= PLATFORM_MAX_PATH))
-               && (strValue.Length() + 1 + dm.GetProjectDataDir().Length() <= MAX_PATH)
-            #endif
-            )
       {
          if (!dm.AssignFile(summaryFileName, strValue, false))
             // Make sure summaryFileName is back to uninitialized state so we can detect problem later.

src/blockfile/PCMAliasBlockFile.cpp

@@ -202,11 +202,8 @@ BlockFile *PCMAliasBlockFile::BuildFromXML(DirManager &dm, const wxChar **attrs)
       const wxString strValue = value;
       if (!wxStricmp(attr, wxT("summaryfile")) && 
             // Can't use XMLValueChecker::IsGoodFileName here, but do part of its test.
-            XMLValueChecker::IsGoodFileString(strValue)
+            XMLValueChecker::IsGoodFileString(strValue) && 
-            #ifdef _WIN32
+            (strValue.Length() + 1 + dm.GetProjectDataDir().Length() <= PLATFORM_MAX_PATH))
-               && (strValue.Length() + 1 + dm.GetProjectDataDir().Length() <= MAX_PATH)
-            #endif
-            )
       {
          if (!dm.AssignFile(summaryFileName, strValue, false))
             // Make sure summaryFileName is back to uninitialized state so we can detect problem later.

src/blockfile/SimpleBlockFile.cpp

@@ -490,11 +490,8 @@ BlockFile *SimpleBlockFile::BuildFromXML(DirManager &dm, const wxChar **attrs)
       const wxString strValue = value;
       if (!wxStricmp(attr, wxT("filename")) && 
             // Can't use XMLValueChecker::IsGoodFileName here, but do part of its test.
-            XMLValueChecker::IsGoodFileString(strValue)
+            XMLValueChecker::IsGoodFileString(strValue) && 
-            #ifdef _WIN32
+            (strValue.Length() + 1 + dm.GetProjectDataDir().Length() <= MAX_PATH))
-               && (strValue.Length() + 1 + dm.GetProjectDataDir().Length() <= MAX_PATH)
-            #endif
-            )
       {
          if (!dm.AssignFile(fileName, strValue, false))
             // Make sure fileName is back to uninitialized state so we can detect problem later.

src/xml/XMLTagHandler.cpp

@@ -41,7 +41,7 @@ bool XMLValueChecker::IsGoodString(const wxString str)
 {
    size_t len = str.Length();
    int nullIndex = str.Find('\0', false);
-   if ((len < 2048) && // Shouldn't be any reason for longer strings, except intentional file corruption.
+   if ((len <= PLATFORM_MAX_PATH) && // Shouldn't be any reason for longer strings, except intentional file corruption.
          (nullIndex == -1)) // No null characters except terminator.
       return true;
    else
@@ -52,14 +52,10 @@ bool XMLValueChecker::IsGoodString(const wxString str)
 bool XMLValueChecker::IsGoodFileName(const wxString strFileName, const wxString strDirName /* = "" */)
 {
    // Test strFileName.
-   if (!IsGoodFileString(strFileName)) 
+   if (!IsGoodFileString(strFileName) || 
+         (strDirName.Length() + 1 + strFileName.Length() > PLATFORM_MAX_PATH))
       return false;
 
-   #ifdef _WIN32
-      if (strFileName.Length() + 1 + strDirName.Length() > MAX_PATH)
-         return false;
-   #endif
-
    // Test the corresponding wxFileName.
    wxFileName fileName(strDirName, strFileName);
    return (fileName.IsOk() && fileName.FileExists());
@@ -83,14 +79,11 @@ bool XMLValueChecker::IsGoodSubdirName(const wxString strSubdirName, const wxStr
    // Note this prevents path separators, and relative path to parents (strDirName), 
    // so fixes vulnerability #3 in the NGS report for UmixIt, 
    // where an attacker could craft an AUP file with relative pathnames to get to system files, for example.
-   if (!IsGoodFileString(strSubdirName) || (strSubdirName == wxT(".")) || (strSubdirName == wxT("..")))
+   if (!IsGoodFileString(strSubdirName) || 
+         (strSubdirName == wxT(".")) || (strSubdirName == wxT("..")) || 
+         (strDirName.Length() + 1 + strSubdirName.Length() > PLATFORM_MAX_PATH))
       return false;
 
-   #ifdef _WIN32
-      if (strSubdirName.Length() + 1 + strDirName.Length() > MAX_PATH)
-         return false;
-   #endif
-
    // Test the corresponding wxFileName.
    wxFileName fileName(strDirName, strSubdirName);
    return (fileName.IsOk() && fileName.DirExists());
@@ -106,12 +99,8 @@ bool XMLValueChecker::IsGoodPathName(const wxString strPathName)
 bool XMLValueChecker::IsGoodPathString(wxString str)
 {
    return (IsGoodString(str) && 
-            !str.IsEmpty() 
+            !str.IsEmpty() && 
-
+            (str.Length() <= PLATFORM_MAX_PATH));
-            #ifdef _WIN32
-               && (str.Length() <= MAX_PATH)
-            #endif
-            );
 }
 
 

win/configwin.h

@@ -1,4 +1,18 @@
-// Microsoft Windows specific include file
+/**********************************************************************
+
+   Audacity: A Digital Audio Editor
+   Audacity(R) is copyright (c) 1999-2011 Audacity Team.
+   License: GPL v2.  See License.txt.
+
+   configwin.h 
+   Dominic Mazzoni, et al
+
+******************************************************************//**
+
+   Microsoft Windows specific include file
+
+*//*******************************************************************/
+
 
 #define MP3SUPPORT 1
 #define USE_FFMPEG 1	//define this to build with ffmpeg import/export