Skip to content

Commit

Permalink
Fix data race in asn1_str2tag() on tntmp which was accidentally made …
Browse files Browse the repository at this point in the history
…static

Variables tntmp and tnst are declared in the same declaration and thus
share storage class specifiers (static). This is unfortunate as tntmp is
used during iteration through tnst array and shouldn't be static.
In particular this leads to two problems that may arise when multiple
threads are executing asn1_str2tag() concurrently:
1. asn1_str2tag() might return value that doesn't correspond to tagstr
   parameter. This can happen if other thread modifies tntmp to point to
   a different tnst element right after a successful name check in the
   if statement.
2. asn1_str2tag() might perform an out-of-bounds read of tnst array.
   This can happen when multiple threads all first execute tntmp = tnst;
   line and then start executing the loop. If that case those threads
   can end up incrementing tntmp past the end of tnst array.

CLA: trivial

Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Shane Lontis <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#26504)
  • Loading branch information
dummyunit authored and t8m committed Jan 23, 2025
1 parent abbc407 commit 7262c0b
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion crypto/asn1/asn1_gen.c
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,8 @@ static int append_exp(tag_exp_arg *arg, int exp_tag, int exp_class,
static int asn1_str2tag(const char *tagstr, int len)
{
unsigned int i;
static const struct tag_name_st *tntmp, tnst[] = {
const struct tag_name_st *tntmp;
static const struct tag_name_st tnst[] = {
ASN1_GEN_STR("BOOL", V_ASN1_BOOLEAN),
ASN1_GEN_STR("BOOLEAN", V_ASN1_BOOLEAN),
ASN1_GEN_STR("NULL", V_ASN1_NULL),
Expand Down

0 comments on commit 7262c0b

Please sign in to comment.