diff options
Diffstat (limited to 'netwerk')
| -rw-r--r-- | netwerk/base/nsStandardURL.cpp | 57 | ||||
| -rw-r--r-- | netwerk/base/nsStandardURL.h | 3 | ||||
| -rw-r--r-- | netwerk/test/gtest/TestStandardURL.cpp | 51 |
3 files changed, 108 insertions, 3 deletions
diff --git a/netwerk/base/nsStandardURL.cpp b/netwerk/base/nsStandardURL.cpp index 1866c10379..3d77093004 100644 --- a/netwerk/base/nsStandardURL.cpp +++ b/netwerk/base/nsStandardURL.cpp @@ -315,6 +315,46 @@ DumpLeakedURLs::~DumpLeakedURLs() } #endif +bool nsStandardURL::IsValid() { + auto checkSegment = [&](const nsStandardURL::URLSegment& aSeg) { + // Bad value + if (NS_WARN_IF(aSeg.mLen < -1)) { + return false; + } + if (aSeg.mLen == -1) { + return true; + } + + // Position outside of string + if (NS_WARN_IF(aSeg.mPos + aSeg.mLen > mSpec.Length())) { + return false; + } + + // Overflow + if (NS_WARN_IF(aSeg.mPos + aSeg.mLen < aSeg.mPos)) { + return false; + } + + return true; + }; + + bool allSegmentsValid = checkSegment(mScheme) && checkSegment(mAuthority) && + checkSegment(mUsername) && checkSegment(mPassword) && + checkSegment(mHost) && checkSegment(mPath) && + checkSegment(mFilepath) && checkSegment(mDirectory) && + checkSegment(mBasename) && checkSegment(mExtension) && + checkSegment(mQuery) && checkSegment(mRef); + if (!allSegmentsValid) { + return false; + } + + if (mScheme.mPos != 0) { + return false; + } + + return true; +} + void nsStandardURL::InitGlobalObjects() { @@ -2752,8 +2792,8 @@ nsStandardURL::SetFilePath(const nsACString &input) mSpec.Cut(mPath.mPos + 1, mFilepath.mLen - 1); // left shift query, and ref ShiftFromQuery(1 - mFilepath.mLen); - // One character for '/', and if we have a query or ref we add their
- // length and one extra for each '?' or '#' characters
+ // One character for '/', and if we have a query or ref we add their + // length and one extra for each '?' or '#' characters mPath.mLen = 1 + (mQuery.mLen >= 0 ? (mQuery.mLen + 1) : 0) + (mRef.mLen >= 0 ? (mRef.mLen + 1) : 0); // these contain only a '/' @@ -3518,6 +3558,10 @@ nsStandardURL::Deserialize(const URIParams& aParams) return false; } + // If we exit early, make sure to clear the URL so we don't fail the sanity + // check in the destructor + auto clearOnExit = MakeScopeExit([&] { Clear(); }); + const StandardURLParams& params = aParams.get_StandardURLParams(); mURLType = params.urlType(); @@ -3565,7 +3609,7 @@ nsStandardURL::Deserialize(const URIParams& aParams) mSupportsFileURL = params.supportsFileURL(); mHostEncoding = params.hostEncoding(); - // Some sanity checks + // Some segment sanity checks NS_ENSURE_TRUE(mScheme.mPos == 0, false); NS_ENSURE_TRUE(mScheme.mLen > 0, false); // Make sure scheme is followed by :// (3 characters) @@ -3577,6 +3621,13 @@ nsStandardURL::Deserialize(const URIParams& aParams) NS_ENSURE_TRUE(mQuery.mLen == -1 || mSpec.CharAt(mQuery.mPos - 1) == '?', false); NS_ENSURE_TRUE(mRef.mLen == -1 || mSpec.CharAt(mRef.mPos - 1) == '#', false); + // Sanity-check the result + if (!IsValid()) { + return false; + } + + clearOnExit.release(); + // mSpecEncoding and mHostA are just caches that can be recovered as needed. return true; } diff --git a/netwerk/base/nsStandardURL.h b/netwerk/base/nsStandardURL.h index eba85528c7..580e1ff262 100644 --- a/netwerk/base/nsStandardURL.h +++ b/netwerk/base/nsStandardURL.h @@ -253,6 +253,9 @@ private: void FindHostLimit(nsACString::const_iterator& aStart, nsACString::const_iterator& aEnd); + // Checks if the URL has a valid representation.
+ bool IsValid();
+ // mSpec contains the normalized version of the URL spec (UTF-8 encoded). nsCString mSpec; int32_t mDefaultPort; diff --git a/netwerk/test/gtest/TestStandardURL.cpp b/netwerk/test/gtest/TestStandardURL.cpp index a013f351cf..44133efcbb 100644 --- a/netwerk/test/gtest/TestStandardURL.cpp +++ b/netwerk/test/gtest/TestStandardURL.cpp @@ -9,6 +9,10 @@ #include "nsComponentManagerUtils.h" #include "nsIIPCSerializableURI.h" #include "mozilla/ipc/URIUtils.h" +#include "mozilla/Unused.h" +#include "nsSerializationHelper.h" +#include "mozilla/Base64.h" +#include "nsEscape.h" TEST(TestStandardURL, Simple) { nsCOMPtr<nsIURL> url( do_CreateInstance(NS_STANDARDURL_CONTRACTID) ); @@ -83,3 +87,50 @@ TEST(TestStandardURL, Deserialize_Bug1392739) nsCOMPtr<nsIIPCSerializableURI> url = do_CreateInstance(NS_STANDARDURL_CID); ASSERT_EQ(url->Deserialize(params), false); } + +TEST(TestStandardURL, CorruptSerialization) +{ + auto spec = "http://user:pass@example.com/path/to/file.ext?query#hash"_ns; + + nsCOMPtr<nsIURI> uri; + nsresult rv = NS_MutateURI(NS_STANDARDURLMUTATOR_CONTRACTID) + .SetSpec(spec) + .Finalize(uri); + ASSERT_EQ(rv, NS_OK); + + nsAutoCString serialization; + nsCOMPtr<nsISerializable> serializable = do_QueryInterface(uri); + ASSERT_TRUE(serializable); + + // Check that the URL is normally serializable. + ASSERT_EQ(NS_OK, NS_SerializeToString(serializable, serialization)); + nsCOMPtr<nsISupports> deserializedObject; + ASSERT_EQ(NS_OK, NS_DeserializeObject(serialization, + getter_AddRefs(deserializedObject))); + + nsAutoCString canonicalBin; + Unused << Base64Decode(serialization, canonicalBin); + +// The spec serialization begins at byte 49 +// If the implementation of nsStandardURL::Write changes, this test will need +// to be adjusted. +#define SPEC_OFFSET 49 + + ASSERT_EQ(Substring(canonicalBin, SPEC_OFFSET, 7), "http://"_ns); + + nsAutoCString corruptedBin = canonicalBin; + // change mScheme.mPos + corruptedBin.BeginWriting()[SPEC_OFFSET + spec.Length()] = 1; + Unused << Base64Encode(corruptedBin, serialization); + ASSERT_EQ( + NS_ERROR_MALFORMED_URI, + NS_DeserializeObject(serialization, getter_AddRefs(deserializedObject))); + + corruptedBin = canonicalBin; + // change mScheme.mLen + corruptedBin.BeginWriting()[SPEC_OFFSET + spec.Length() + 4] = 127; + Unused << Base64Encode(corruptedBin, serialization); + ASSERT_EQ( + NS_ERROR_MALFORMED_URI, + NS_DeserializeObject(serialization, getter_AddRefs(deserializedObject))); +}
\ No newline at end of file |