Commit eee95522 authored by Pauli's avatar Pauli
Browse files

Bounds check string functions in apps.


This includes strcat, strcpy and sprintf.

In the x509 app, the code has been cleaned up as well.

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3868)
parent 67fdc998
Loading
Loading
Loading
Loading
+5 −5
Original line number Diff line number Diff line
@@ -312,7 +312,7 @@ int enc_main(int argc, char **argv)
            for (;;) {
                char prompt[200];

                sprintf(prompt, "enter %s %s password:",
                BIO_snprintf(prompt, sizeof(prompt), "enter %s %s password:",
                        OBJ_nid2ln(EVP_CIPHER_nid(cipher)),
                        (enc) ? "encryption" : "decryption");
                strbuf[0] = '\0';
@@ -565,7 +565,7 @@ int enc_main(int argc, char **argv)
#endif
    release_engine(e);
    OPENSSL_free(pass);
    return (ret);
    return ret;
}

static void show_ciphers(const OBJ_NAME *name, void *arg)
@@ -599,7 +599,7 @@ static int set_hex(char *in, unsigned char *out, int size)
    n = strlen(in);
    if (n > (size * 2)) {
        BIO_printf(bio_err, "hex string is too long\n");
        return (0);
        return 0;
    }
    memset(out, 0, size);
    for (i = 0; i < n; i++) {
@@ -609,7 +609,7 @@ static int set_hex(char *in, unsigned char *out, int size)
            break;
        if (!isxdigit(j)) {
            BIO_printf(bio_err, "non-hex digit\n");
            return (0);
            return 0;
        }
        j = (unsigned char)OPENSSL_hexchar2int(j);
        if (i & 1)
@@ -617,5 +617,5 @@ static int set_hex(char *in, unsigned char *out, int size)
        else
            out[i / 2] = (j << 4);
    }
    return (1);
    return 1;
}
+5 −3
Original line number Diff line number Diff line
@@ -27,6 +27,8 @@ NON_EMPTY_TRANSLATION_UNIT
# define CLCERTS         0x8
# define CACERTS         0x10

#define PASSWD_BUF_SIZE 2048

static int get_cert_chain(X509 *cert, X509_STORE *store,
                          STACK_OF(X509) **chain);
int dump_certs_keys_p12(BIO *out, const PKCS12 *p12,
@@ -119,7 +121,7 @@ int pkcs12_main(int argc, char **argv)
{
    char *infile = NULL, *outfile = NULL, *keyname = NULL, *certfile = NULL;
    char *name = NULL, *csp_name = NULL;
    char pass[2048] = "", macpass[2048] = "";
    char pass[PASSWD_BUF_SIZE] = "", macpass[PASSWD_BUF_SIZE] = "";
    int export_cert = 0, options = 0, chain = 0, twopass = 0, keytype = 0;
    int iter = PKCS12_DEFAULT_ITER, maciter = PKCS12_DEFAULT_ITER;
# ifndef OPENSSL_NO_RC2
@@ -455,7 +457,7 @@ int pkcs12_main(int argc, char **argv)
        }

        if (!twopass)
            strcpy(macpass, pass);
            OPENSSL_strlcpy(macpass, pass, sizeof(macpass));

        p12 = PKCS12_create(cpass, name, key, ucert, certs,
                            key_pbe, cert_pbe, iter, -1, keytype);
@@ -583,7 +585,7 @@ int pkcs12_main(int argc, char **argv)
    OPENSSL_free(badpass);
    OPENSSL_free(passin);
    OPENSSL_free(passout);
    return (ret);
    return ret;
}

int dump_certs_keys_p12(BIO *out, const PKCS12 *p12, const char *pass,
+17 −11
Original line number Diff line number Diff line
@@ -49,7 +49,13 @@

static SSL *doConnection(SSL *scon, const char *host, SSL_CTX *ctx);

/*
 * Define a HTTP get command globally.
 * Also define the size of the command, this is two bytes less than
 * the size of the string because the %s is replaced by the URL.
 */
static const char fmt_http_get_cmd[] = "GET %s HTTP/1.0\r\n\r\n";
static const size_t fmt_http_get_cmd_size = sizeof(fmt_http_get_cmd) - 2;

typedef enum OPTION_choice {
    OPT_ERR = -1, OPT_EOF = 0, OPT_HELP,
@@ -173,7 +179,7 @@ int s_time_main(int argc, char **argv)
            break;
        case OPT_WWW:
            www_path = opt_arg();
            buf_size = strlen(www_path) + sizeof(fmt_http_get_cmd);
            buf_size = strlen(www_path) + fmt_http_get_cmd_size;
            if (buf_size > sizeof(buf)) {
                BIO_printf(bio_err, "%s: -www option is too long\n", prog);
                goto end;
@@ -230,9 +236,9 @@ int s_time_main(int argc, char **argv)
            goto end;

        if (www_path != NULL) {
            sprintf(buf, fmt_http_get_cmd, www_path);
            buf_len = strlen(buf);
            if (SSL_write(scon, buf, buf_len) <= 0)
            buf_len = BIO_snprintf(buf, sizeof(buf), fmt_http_get_cmd,
                                   www_path);
            if (buf_len <= 0 || SSL_write(scon, buf, buf_len) <= 0)
                goto end;
            while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
                bytes_read += i;
@@ -288,9 +294,8 @@ int s_time_main(int argc, char **argv)
    }

    if (www_path != NULL) {
        sprintf(buf, fmt_http_get_cmd, www_path);
        buf_len = strlen(buf);
        if (SSL_write(scon, buf, buf_len) <= 0)
        buf_len = BIO_snprintf(buf, sizeof(buf), fmt_http_get_cmd, www_path);
        if (buf_len <= 0 || SSL_write(scon, buf, buf_len) <= 0)
            goto end;
        while (SSL_read(scon, buf, sizeof(buf)) > 0)
            continue;
@@ -319,8 +324,9 @@ int s_time_main(int argc, char **argv)
            goto end;

        if (www_path != NULL) {
            sprintf(buf, "GET %s HTTP/1.0\r\n\r\n", www_path);
            if (SSL_write(scon, buf, strlen(buf)) <= 0)
            buf_len = BIO_snprintf(buf, sizeof(buf), fmt_http_get_cmd,
                                   www_path);
            if (buf_len <= 0 || SSL_write(scon, buf, buf_len) <= 0)
                goto end;
            while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
                bytes_read += i;
@@ -361,7 +367,7 @@ int s_time_main(int argc, char **argv)
 end:
    SSL_free(scon);
    SSL_CTX_free(ctx);
    return (ret);
    return ret;
}

/*-
@@ -375,7 +381,7 @@ static SSL *doConnection(SSL *scon, const char *host, SSL_CTX *ctx)
    fd_set readfds;

    if ((conn = BIO_new(BIO_s_connect())) == NULL)
        return (NULL);
        return NULL;

    BIO_set_conn_hostname(conn, host);

+13 −20
Original line number Diff line number Diff line
@@ -890,34 +890,27 @@ int x509_main(int argc, char **argv)
    ASN1_OBJECT_free(objtmp);
    release_engine(e);
    OPENSSL_free(passin);
    return (ret);
    return ret;
}

static ASN1_INTEGER *x509_load_serial(const char *CAfile, const char *serialfile,
                                      int create)
static ASN1_INTEGER *x509_load_serial(const char *CAfile,
                                      const char *serialfile, int create)
{
    char *buf = NULL, *p;
    char *buf = NULL;
    ASN1_INTEGER *bs = NULL;
    BIGNUM *serial = NULL;
    size_t len;

    len = ((serialfile == NULL)
           ? (strlen(CAfile) + strlen(POSTFIX) + 1)
           : (strlen(serialfile))) + 1;
    buf = app_malloc(len, "serial# buffer");
    if (serialfile == NULL) {
        strcpy(buf, CAfile);
        for (p = buf; *p; p++)
            if (*p == '.') {
                *p = '\0';
                break;
            }
        strcat(buf, POSTFIX);
    } else {
        strcpy(buf, serialfile);
        const char *p = strchr(CAfile, '.');
        size_t len = p != NULL ? (size_t)(p - CAfile) : strlen(CAfile);

        buf = app_malloc(len + sizeof(POSTFIX), "serial# buffer");
        memcpy(buf, CAfile, len);
        memcpy(buf + len, POSTFIX, sizeof(POSTFIX));
        serialfile = buf;
    }

    serial = load_serial(buf, create, NULL);
    serial = load_serial(serialfile, create, NULL);
    if (serial == NULL)
        goto end;

@@ -926,7 +919,7 @@ static ASN1_INTEGER *x509_load_serial(const char *CAfile, const char *serialfile
        goto end;
    }

    if (!save_serial(buf, NULL, serial, &bs))
    if (!save_serial(serialfile, NULL, serial, &bs))
        goto end;

 end: