Closed Bug 1538980 Opened 6 years ago Closed 5 years ago

SECU_ReadDERFromFile calls strstr on a string that isn't guaranteed to be null-terminated

Categories

(NSS :: Tools, enhancement, P1)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: keeler, Assigned: keeler)

References

Details

(Keywords: csectype-dos, sec-low)

Attachments

(1 file)

SECStatus
SECU_ReadDERFromFile(SECItem *der, PRFileDesc *inFile, PRBool ascii,
                     PRBool warnOnPrivateKeyInAsciiFile)
{
    SECStatus rv;
    if (ascii) {
        /* First convert ascii to binary */
        SECItem filedata;
        char *asc, *body;

        /* Read in ascii data */
        rv = SECU_FileToItem(&filedata, inFile);
        if (rv != SECSuccess)
            return rv;
        asc = (char *)filedata.data;
        if (!asc) {
            fprintf(stderr, "unable to read data from input file\n");
            return SECFailure;
        }

        if (warnOnPrivateKeyInAsciiFile && strstr(asc, "PRIVATE KEY")) {
            fprintf(stderr, "Warning: ignoring private key. Consider to use "
                            "pk12util.\n");
        }

        /* check for headers and trailers and remove them */
        if ((body = strstr(asc, "-----BEGIN")) != NULL) {
            char *trailer = NULL;
            asc = body;
            body = PORT_Strchr(body, '\n');
            if (!body)
                body = PORT_Strchr(asc, '\r'); /* maybe this is a MAC file */
            if (body)
                trailer = strstr(++body, "-----END");

asc and body point (in) to filedata.data, which gets filled out in SECU_FileToItem and is just the contents of the file. There is no terminating null character. So when SECU_ReadDERFromFile calls strstr and the search term isn't present in the file, this will just read past the end of the space allocated for filedata.data.

I think this is just potential crash/dos, but this can be used to write a zero byte out of bounds, so it might be exploitable (the byte has to be right before the string "-----END", so I imagine this would have to be used in combination with another vulnerability).

Priority: -- → P2
Assignee: nobody → dkeeler
Priority: P2 → P1
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.51
Group: crypto-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: