kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fix bugs in X509_NAME_add_entry.

Browse Source 
|set| should be evaluated to determine whether to insert/append before
it is reused as a temporary variable.

When incrementing the |set| of X509_NAME_ENTRY, the inserted entry
should not be incremented.

Thanks to Ingo Schwarze for extensive debugging and the initial
fix.

(Imported from upstream bbf27cd58337116c57a1c942153330ff83d5540a)

Change-Id: Ib45d92fc6d52d7490b01d3c475eafc42dd6ef721
Reviewed-on: https://boringssl-review.googlesource.com/28005
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
This commit is contained in:
Steven Valdez 2018-05-03 13:15:12 -04:00 committed by CQ bot account: commit-bot@chromium.org
parent 0c9ac2e7bf
commit dd444b1d8e
2 changed files with 26 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
crypto/x509/x509_test.cc
View File

@ -1259,3 +1259,27 @@ TEST(X509Test, PrettyPrintIntegers) {
}
}
}
TEST(X509Test, X509NameSet) {
bssl::UniquePtr<X509_NAME> name(X509_NAME_new());
EXPECT_TRUE(X509_NAME_add_entry_by_txt(
name.get(), "C", MBSTRING_ASC, reinterpret_cast<const uint8_t *>("US"),
-1, -1, 0));
EXPECT_EQ(X509_NAME_entry_count(name.get()), 1);
EXPECT_TRUE(X509_NAME_add_entry_by_txt(
name.get(), "C", MBSTRING_ASC, reinterpret_cast<const uint8_t *>("CA"),
-1, -1, 0));
EXPECT_EQ(X509_NAME_entry_count(name.get()), 2);
EXPECT_TRUE(X509_NAME_add_entry_by_txt(
name.get(), "C", MBSTRING_ASC, reinterpret_cast<const uint8_t *>("UK"),
-1, -1, 0));
EXPECT_EQ(X509_NAME_entry_count(name.get()), 3);
EXPECT_TRUE(X509_NAME_add_entry_by_txt(
name.get(), "C", MBSTRING_ASC, reinterpret_cast<const uint8_t *>("JP"),
-1, 1, 0));
EXPECT_EQ(X509_NAME_entry_count(name.get()), 4);
// Check that the correct entries get incremented when inserting new entry.
EXPECT_EQ(X509_NAME_ENTRY_set(X509_NAME_get_entry(name.get(), 1)), 1);
EXPECT_EQ(X509_NAME_ENTRY_set(X509_NAME_get_entry(name.get(), 2)), 2);
}

5
crypto/x509/x509name.c
View File

@ -238,6 +238,7 @@ int X509_NAME_add_entry(X509_NAME *name, X509_NAME_ENTRY *ne, int loc,
else if (loc < 0)
loc = n;
inc = (set == 0);
name->modified = 1;
if (set == -1) {
@ -246,7 +247,6 @@ int X509_NAME_add_entry(X509_NAME *name, X509_NAME_ENTRY *ne, int loc,
inc = 1;
} else {
set = sk_X509_NAME_ENTRY_value(sk, loc - 1)->set;
inc = 0;
}
} else { /* if (set >= 0) */
@ -257,7 +257,6 @@ int X509_NAME_add_entry(X509_NAME *name, X509_NAME_ENTRY *ne, int loc,
set = 0;
} else
set = sk_X509_NAME_ENTRY_value(sk, loc)->set;
inc = (set == 0) ? 1 : 0;
}
if ((new_name = X509_NAME_ENTRY_dup(ne)) == NULL)
@ -270,7 +269,7 @@ int X509_NAME_add_entry(X509_NAME *name, X509_NAME_ENTRY *ne, int loc,
if (inc) {
n = sk_X509_NAME_ENTRY_num(sk);
for (i = loc + 1; i < n; i++)
sk_X509_NAME_ENTRY_value(sk, i - 1)->set += 1;
sk_X509_NAME_ENTRY_value(sk, i)->set += 1;
}
return (1);
err: