From 22c71d753298e8a781cd90ac28bc2ce0aeb53997 Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Sat, 27 Sep 2014 14:51:19 -0400 Subject: [PATCH] bug 1073747 - nsurlparser xpcom methods don't validate input r=sworkman --- netwerk/base/src/nsURLParsers.cpp | 36 +++++++++++++++++++++++-------- netwerk/test/unit/test_1073747.js | 30 ++++++++++++++++++++++++++ netwerk/test/unit/xpcshell.ini | 1 + 3 files changed, 58 insertions(+), 9 deletions(-) create mode 100644 netwerk/test/unit/test_1073747.js diff --git a/netwerk/base/src/nsURLParsers.cpp b/netwerk/base/src/nsURLParsers.cpp index 81aadd345f2..37f3e722c72 100644 --- a/netwerk/base/src/nsURLParsers.cpp +++ b/netwerk/base/src/nsURLParsers.cpp @@ -52,7 +52,9 @@ nsBaseURLParser::ParseURL(const char *spec, int32_t specLen, uint32_t *authorityPos, int32_t *authorityLen, uint32_t *pathPos, int32_t *pathLen) { - NS_PRECONDITION(spec, "null pointer"); + if (NS_WARN_IF(!spec)) { + return NS_ERROR_INVALID_POINTER; + } if (specLen < 0) specLen = strlen(spec); @@ -165,7 +167,9 @@ nsBaseURLParser::ParseAuthority(const char *auth, int32_t authLen, uint32_t *hostnamePos, int32_t *hostnameLen, int32_t *port) { - NS_PRECONDITION(auth, "null pointer"); + if (NS_WARN_IF(!auth)) { + return NS_ERROR_INVALID_POINTER; + } if (authLen < 0) authLen = strlen(auth); @@ -205,7 +209,9 @@ nsBaseURLParser::ParsePath(const char *path, int32_t pathLen, uint32_t *queryPos, int32_t *queryLen, uint32_t *refPos, int32_t *refLen) { - NS_PRECONDITION(path, "null pointer"); + if (NS_WARN_IF(!path)) { + return NS_ERROR_INVALID_POINTER; + } if (pathLen < 0) pathLen = strlen(path); @@ -266,7 +272,9 @@ nsBaseURLParser::ParseFilePath(const char *filepath, int32_t filepathLen, uint32_t *basenamePos, int32_t *basenameLen, uint32_t *extensionPos, int32_t *extensionLen) { - NS_PRECONDITION(filepath, "null pointer"); + if (NS_WARN_IF(!filepath)) { + return NS_ERROR_INVALID_POINTER; + } if (filepathLen < 0) filepathLen = strlen(filepath); @@ -312,7 +320,9 @@ nsBaseURLParser::ParseFileName(const char *filename, int32_t filenameLen, uint32_t *basenamePos, int32_t *basenameLen, uint32_t *extensionPos, int32_t *extensionLen) { - NS_PRECONDITION(filename, "null pointer"); + if (NS_WARN_IF(!filename)) { + return NS_ERROR_INVALID_POINTER; + } if (filenameLen < 0) filenameLen = strlen(filename); @@ -408,7 +418,9 @@ nsNoAuthURLParser::ParseFilePath(const char *filepath, int32_t filepathLen, uint32_t *basenamePos, int32_t *basenameLen, uint32_t *extensionPos, int32_t *extensionLen) { - NS_PRECONDITION(filepath, "null pointer"); + if (NS_WARN_IF(!filepath)) { + return NS_ERROR_INVALID_POINTER; + } if (filepathLen < 0) filepathLen = strlen(filepath); @@ -450,7 +462,9 @@ nsAuthURLParser::ParseAuthority(const char *auth, int32_t authLen, { nsresult rv; - NS_PRECONDITION(auth, "null pointer"); + if (NS_WARN_IF(!auth)) { + return NS_ERROR_INVALID_POINTER; + } if (authLen < 0) authLen = strlen(auth); @@ -498,7 +512,9 @@ nsAuthURLParser::ParseUserInfo(const char *userinfo, int32_t userinfoLen, uint32_t *usernamePos, int32_t *usernameLen, uint32_t *passwordPos, int32_t *passwordLen) { - NS_PRECONDITION(userinfo, "null pointer"); + if (NS_WARN_IF(!userinfo)) { + return NS_ERROR_INVALID_POINTER; + } if (userinfoLen < 0) userinfoLen = strlen(userinfo); @@ -532,7 +548,9 @@ nsAuthURLParser::ParseServerInfo(const char *serverinfo, int32_t serverinfoLen, uint32_t *hostnamePos, int32_t *hostnameLen, int32_t *port) { - NS_PRECONDITION(serverinfo, "null pointer"); + if (NS_WARN_IF(!serverinfo)) { + return NS_ERROR_INVALID_POINTER; + } if (serverinfoLen < 0) serverinfoLen = strlen(serverinfo); diff --git a/netwerk/test/unit/test_1073747.js b/netwerk/test/unit/test_1073747.js new file mode 100644 index 00000000000..c930514e7e4 --- /dev/null +++ b/netwerk/test/unit/test_1073747.js @@ -0,0 +1,30 @@ +// Test based on submitted one from Peter B Shalimoff + +var test = function(s, funcName){ + function Arg(){}; + Arg.prototype.toString = function(){ + do_print("Testing " + funcName + " with null args"); + return this.value; + }; + // create a generic arg lits of null, -1, and 10 nulls + var args = [s, -1]; + for (var i = 0; i < 10; ++i) { + args.push(new Arg()); + } + var up = Components.classes["@mozilla.org/network/url-parser;1?auth=maybe"].getService(Components.interfaces.nsIURLParser); + try { + up[funcName].apply(up, args); + return args; + } catch (x) { + do_check_true(true); // make sure it throws an exception instead of crashing + return x; + } + // should always have an exception to catch + do_check_true(false); +}; +var s = null; +var funcs = ["parseAuthority", "parseFileName", "parseFilePath", "parsePath", "parseServerInfo", "parseURL", "parseUserInfo"]; + +function run_test() { + funcs.forEach(function(f){test(s, f);}); +} diff --git a/netwerk/test/unit/xpcshell.ini b/netwerk/test/unit/xpcshell.ini index 0f4c5903afa..07f7ba67399 100644 --- a/netwerk/test/unit/xpcshell.ini +++ b/netwerk/test/unit/xpcshell.ini @@ -303,3 +303,4 @@ run-if = os == "win" [test_tls_server.js] # The local cert service used by this test is not currently shipped on Android skip-if = os == "android" +[test_1073747.js]