Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1005256: Improve parameter validation in mozilla::pkix::der::Inpu…
…t::GetSECItem, r=mmc

--HG--
extra : rebase_source : 93b65e103c86747ddaf463e639aacffdf7ccb08f
extra : histedit_source : 10ef0ab13fb9de710ea3c589600db4632f9cf4a0
  • Loading branch information
briansmith committed May 2, 2014
1 parent e147694 commit bf9ed48
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 12 deletions.
21 changes: 12 additions & 9 deletions lib/mozpkix/lib/pkixder.h
Expand Up @@ -183,21 +183,24 @@ class Input
{
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<uint8_t*>(mark.mMark);
// TODO: Return false if bounds check fails
item.len = input - mark.mMark;
return true;
item.data = const_cast<uint8_t*>(mark.mark);
item.len = static_cast<decltype(item.len)>(input - mark.mark);
return Success;
}

private:
Expand Down
8 changes: 6 additions & 2 deletions lib/mozpkix/lib/pkixocsp.cpp
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
}
Expand Down
21 changes: 20 additions & 1 deletion lib/mozpkix/test/gtest/pkixder_input_tests.cpp
Expand Up @@ -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;
Expand Down

0 comments on commit bf9ed48

Please sign in to comment.