Validate asn date based on position of Z

[anhu]

Fixes #8597

[anhu]

jenkins retest this please
(Java crashed.)

[anhu]

Jenkins retest this please

[anhu]

jenkins retest this please

[JacobBarthelmeh]

Needs a test case

[JacobBarthelmeh]

Needs a test case

[anhu]

It will always eventually succeed. Need to redesign this part so we actually record whether it was generalized or utc.

        if (GetTimeString(crl->crlList->lastDate, ASN_UTC_TIME,
            tmp, MAX_WIDTH) != WOLFSSL_SUCCESS) {
            if (GetTimeString(crl->crlList->lastDate, ASN_GENERALIZED_TIME,
            tmp, MAX_WIDTH) != WOLFSSL_SUCCESS) {
                WOLFSSL_MSG("Error getting last update date");
                return WOLFSSL_FAILURE;
            }
        }

[anhu]

Please do not merge. I would like to squash this once it is approved.

[anhu]

Jenkins retest this please

[douzzer]

I don't see any guard to prevent an out-of-bounds read here, which would result in undefined behavior.

GetTimeString() is passed a len arg, but does not pass it on to ExtractDate(), and does not do any bounds checking of its own before passing date to ExtractDate().

Note that the unsafe logic can be reached from public API wolfSSL_ASN1_TIME_to_string(), also with no relevant bounds protection, so this is really not OK on that basis alone.

Bottom line, a refactor is needed to pass the len to ExtractDate() and use it for bounds checking.

Also note that the existing calls in ExtractDate() to btoi() and GetTime() are unsafe if date is not null-terminated. It is never safe to handle a non-null-terminated string without having and honoring an explicit length, but worse, there is no way to use btoi() and GetTime() safely on non-null-terminated strings.

[anhu]

@douzzer ,

Not sure how we can resolve this. Note the following:

int wc_ValidateDate(const byte* date, byte format, int dateType)
...
    if (!ExtractDate(date, format, &certTime, &i)) {
        WOLFSSL_MSG("Error extracting the date");
        return 0;
    }

No len is passed in to wc_ValidateDate(). It is a wolfcrypt local function, but it is macrofied to XVALIDATE_DATE . I think in order to get the level of safety you are looking for we need a major refactor which would fall way out of the scope of this PR.

I suggest your proposal be done in another PR. Let me know your thoughts.