Update of /cvsroot/audacity/audacity-src/src/xml
In directory sc8-pr-cvs11.sourceforge.net:/tmp/cvs-serv7914/xml
Modified Files:
Tag: Audacity_UmixIt
XMLTagHandler.cpp XMLTagHandler.h
Log Message:
Test XML input per NGS report for UmixIt.
Index: XMLTagHandler.h
===================================================================
RCS file: /cvsroot/audacity/audacity-src/src/xml/XMLTagHandler.h,v
retrieving revision 1.4.6.1
retrieving revision 1.4.6.2
diff -u -d -r1.4.6.1 -r1.4.6.2
--- XMLTagHandler.h 12 Dec 2006 03:27:12 -0000 1.4.6.1
+++ XMLTagHandler.h 17 Dec 2006 05:34:39 -0000 1.4.6.2
@@ -18,11 +18,25 @@
#ifndef __AUDACITY_XML_TAG_HANDLER__
#define __AUDACITY_XML_TAG_HANDLER__
-// "Good" means the name is well-formed and names an existing file.
-// These are functions rather than methods because some non-descendants of
XMLTagHandler need it. //vvvvv necessarily?
-bool IsGoodSubdirNameFromXML(const wxString strSubdirName, const wxString
strDirName = "");
-bool IsGoodFileNameFromXML(const wxString strFileName, const wxString
strDirName = "");
-bool IsGoodPathNameFromXML(const wxString strPathName);
+class XMLValueChecker
+{
+public:
+ // "Good" means well-formed and for the file-related functions, names an
existing file or folder.
+ // They are used in HandleXMLTag and BuildFomXML methods to check the input
for
+ // security vulnerabilites, per the NGS report for UmixIt.
+ static bool IsGoodString(const wxString str);
+
+ static bool IsGoodFileName(const wxString strFileName, const wxString
strDirName = "");
+ static bool IsGoodSubdirName(const wxString strSubdirName, const wxString
strDirName = "");
+ static bool IsGoodPathName(const wxString strPathName);
+
+ // Note that because wxString::ToLong does additional testing, IsGoodInt
doesn't duplicate
+ // that testing, so use wxString::ToLong, not just atoi.
+ static bool IsGoodInt(const wxString strInt);
+
+private:
+ static bool IsGoodFileString(wxString str);
+};
class XMLTagHandler {
public:
Index: XMLTagHandler.cpp
===================================================================
RCS file: /cvsroot/audacity/audacity-src/src/xml/XMLTagHandler.cpp,v
retrieving revision 1.4.6.1
retrieving revision 1.4.6.2
diff -u -d -r1.4.6.1 -r1.4.6.2
--- XMLTagHandler.cpp 12 Dec 2006 03:27:12 -0000 1.4.6.1
+++ XMLTagHandler.cpp 17 Dec 2006 05:34:39 -0000 1.4.6.2
@@ -18,51 +18,97 @@
#include "../Audacity.h"
#include "../Internat.h"
+#ifdef _WIN32
+ #include <windows.h>
+#endif
+
#include <wx/defs.h>
#include <wx/filename.h>
-// "Good" means the name is well-formed and names an existing file.
-// These are functions rather than methods because some non-descendants of
XMLTagHandler need it. //vvvvv necessarily?
-bool IsGoodFileString(wxString str)
+
+bool XMLValueChecker::IsGoodString(const wxString str)
{
- // Test against corrupt filenames per security vulnerability report by NGS
for UmixIt.
- wxString intlStrFileName = FILENAME(str);
- size_t len = intlStrFileName.Length();
- return ((len <= 128) && // FILENAME_MAX is 260 in MSVC, but inconsistent
across platforms, sometimes huge.
- (intlStrFileName.Find('\0') == len) && // No null characters
except terminator.
- (intlStrFileName.Find(wxFileName::GetPathSeparator()) == -1)); //
No path separator characters. //vvv (this won't work on CVS HEAD)
+ size_t len = str.Length();
+ int nullIndex = str.Find('\0');
+ if ((len < 2048) && // Shouldn't be any reason for longer strings, except
intentional file corruption.
+ (nullIndex > -1) && // _Should_ always find it, at string terminator.
+ (nullIndex == len)) // No null characters except terminator.
+ return true;
+ else
+ return false; // good place for a breakpoint
}
-bool IsGoodSubdirNameFromXML(const wxString strSubdirName, const wxString
strDirName /* = "" */)
+// "Good" means the name is well-formed and names an existing file or folder.
+bool XMLValueChecker::IsGoodFileName(const wxString strFileName, const
wxString strDirName /* = "" */)
{
- // Test strSubdirName.
- if (!IsGoodFileString(strSubdirName)) return false;
+ // Test strFileName.
+ if (!IsGoodFileString(strFileName))
+ return false;
+
+ #ifdef _WIN32
+ if (strFileName.Length() + 1 + strDirName.Length() > MAX_PATH)
+ return false;
+ #endif
+
+ // Test the corresponding wxFileName.
+ wxFileName fileName(FILENAME(strDirName), FILENAME(strFileName));
+ return (fileName.IsOk() && fileName.FileExists());
+}
+
+bool XMLValueChecker::IsGoodSubdirName(const wxString strSubdirName, const
wxString strDirName /* = "" */)
+{
+ // Test strSubdirName.
+ // Note this prevents path separators, 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))
+ return false;
+
+ #ifdef _WIN32
+ if (strSubdirName.Length() + 1 + strDirName.Length() > MAX_PATH)
+ return false;
+ #endif
// Test the corresponding wxFileName.
wxFileName fileName(FILENAME(strDirName), FILENAME(strSubdirName));
return (fileName.IsOk() && fileName.DirExists());
}
-bool IsGoodFileNameFromXML(const wxString strFileName, const wxString
strDirName /* = "" */)
+bool XMLValueChecker::IsGoodPathName(const wxString strPathName)
{
- // Test strFileName.
- if (!IsGoodFileString(strFileName)) return false;
-
// Test the corresponding wxFileName.
- wxFileName fileName(FILENAME(strDirName), FILENAME(strFileName));
- return (fileName.IsOk() && fileName.FileExists());
+ wxFileName fileName(FILENAME(strPathName));
+ return XMLValueChecker::IsGoodFileName(fileName.GetFullName(),
fileName.GetPath(wxPATH_GET_VOLUME));
}
-bool IsGoodPathNameFromXML(const wxString strPathName)
+bool XMLValueChecker::IsGoodFileString(wxString str)
{
- // Test strPathName.
- wxString intlStrPathName = FILENAME(strPathName);
- if ((intlStrPathName.Find('\0') < intlStrPathName.Length())) // No null
characters.
+ return (IsGoodString(str) &&
+ (str.Length() <= 260) && // FILENAME_MAX is 260 in MSVC, but
inconsistent across platforms, sometimes huge.
+ (str.Find(wxFileName::GetPathSeparator()) == -1)); // No path
separator characters. //vvv (this won't work on CVS HEAD)
+}
+
+bool XMLValueChecker::IsGoodInt(const wxString strInt)
+{
+ if (!IsGoodString(strInt))
return false;
- // Test the corresponding wxFileName.
- wxFileName fileName(intlStrPathName);
- return IsGoodFileNameFromXML(fileName.GetFullName(),
fileName.GetPath(wxPATH_GET_VOLUME));
+ // Check that the value won't overflow.
+ const wxString strMAXINT = "2147483647";
+ size_t lenMAXINT = strMAXINT.Length();
+ if (strInt.Length() > lenMAXINT)
+ return false;
+ else if (strInt.Length() == lenMAXINT)
+ {
+ const int digitsMAXINT[] = {2, 1, 4, 7, 4, 8, 3, 6, 4, 7};
+ unsigned long nTest;
+ wxString strTest;
+ for (unsigned int i = 0; i < lenMAXINT; i++) {
+ strTest = strInt[i];
+ if (!strTest.ToULong(&nTest) || (nTest > digitsMAXINT[i]))
+ return false;
+ }
+ }
+ return true;
}
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Audacity-cvs mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/audacity-cvs
