diff --git a/security/pkix/lib/pkixder.h b/security/pkix/lib/pkixder.h index cd145af7b9c2..bd2628c7e250 100644 --- a/security/pkix/lib/pkixder.h +++ b/security/pkix/lib/pkixder.h @@ -183,21 +183,24 @@ public: { private: friend class Input; - explicit Mark(const uint8_t* mark) : mMark(mark) { } - const uint8_t* const mMark; + Mark(const Input& input, const uint8_t* mark) : input(input), mark(mark) { } + const Input& input; + const uint8_t* const mark; void operator=(const Mark&) /* = delete */; }; - Mark GetMark() const { return Mark(input); } + Mark GetMark() const { return Mark(*this, input); } - bool GetSECItem(SECItemType type, const Mark& mark, /*out*/ SECItem& item) + Result GetSECItem(SECItemType type, const Mark& mark, /*out*/ SECItem& item) { - PR_ASSERT(mark.mMark < input); + if (&mark.input != this || mark.mark > input) { + PR_NOT_REACHED("invalid mark"); + return Fail(SEC_ERROR_INVALID_ARGS); + } item.type = type; - item.data = const_cast(mark.mMark); - // TODO: Return false if bounds check fails - item.len = input - mark.mMark; - return true; + item.data = const_cast(mark.mark); + item.len = static_cast(input - mark.mark); + return Success; } private: diff --git a/security/pkix/lib/pkixocsp.cpp b/security/pkix/lib/pkixocsp.cpp index 68b800267c0d..35bbcc2b2f22 100644 --- a/security/pkix/lib/pkixocsp.cpp +++ b/security/pkix/lib/pkixocsp.cpp @@ -441,7 +441,9 @@ BasicResponse(der::Input& input, Context& context) CERTSignedData signedData; - input.GetSECItem(siBuffer, mark, signedData.data); + if (input.GetSECItem(siBuffer, mark, signedData.data) != der::Success) { + return der::Failure; + } if (der::Nested(input, der::SEQUENCE, bind(der::AlgorithmIdentifier, _1, @@ -502,7 +504,9 @@ BasicResponse(der::Input& input, Context& context) return der::Failure; } - input.GetSECItem(siBuffer, mark, certs[numCerts]); + if (input.GetSECItem(siBuffer, mark, certs[numCerts]) != der::Success) { + return der::Failure; + } ++numCerts; } } diff --git a/security/pkix/test/gtest/pkixder_input_tests.cpp b/security/pkix/test/gtest/pkixder_input_tests.cpp index ab8b9c81b798..8d50b750cc5e 100644 --- a/security/pkix/test/gtest/pkixder_input_tests.cpp +++ b/security/pkix/test/gtest/pkixder_input_tests.cpp @@ -417,13 +417,32 @@ TEST_F(pkixder_input_tests, MarkAndGetSECItem) SECItem item; memset(&item, 0x00, sizeof item); - ASSERT_TRUE(input.GetSECItem(siBuffer, mark, item)); + ASSERT_EQ(Success, input.GetSECItem(siBuffer, mark, item)); ASSERT_EQ(siBuffer, item.type); ASSERT_EQ(sizeof expectedItemData, item.len); ASSERT_TRUE(item.data); ASSERT_EQ(0, memcmp(item.data, expectedItemData, sizeof expectedItemData)); } +// Cannot run this test on debug builds because of the PR_NOT_REACHED +#ifndef DEBUG +TEST_F(pkixder_input_tests, MarkAndGetSECItemDifferentInput) +{ + Input input; + const uint8_t der[] = { 0x11, 0x22, 0x33, 0x44 }; + ASSERT_EQ(Success, input.Init(der, sizeof der)); + + Input another; + Input::Mark mark = another.GetMark(); + + ASSERT_EQ(Success, input.Skip(3)); + + SECItem item; + ASSERT_EQ(Failure, input.GetSECItem(siBuffer, mark, item)); + ASSERT_EQ(SEC_ERROR_INVALID_ARGS, PR_GetError()); +} +#endif + TEST_F(pkixder_input_tests, ExpectTagAndLength) { Input input;