Don't mutate the buffer in PEM_get_EVP_CIPHER_INFO
There's a lot more we could improve in this function, but fix this
particular egregious issue first.
Change-Id: Idc4b7f9aa62972293ead4f8534b8461942318e21
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/74808
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/pem/internal.h b/crypto/pem/internal.h
index 3cfc8ac..9fe9a5b 100644
--- a/crypto/pem/internal.h
+++ b/crypto/pem/internal.h
@@ -22,10 +22,7 @@
// error. |header| must be a NUL-terminated string. If |header| does not
// specify encryption, this function will return success and set
// |cipher->cipher| to NULL.
-//
-// WARNING: This function will internally write to the string pointed by
-// |header|. |header| must not point to constant storage.
-int PEM_get_EVP_CIPHER_INFO(char *header, EVP_CIPHER_INFO *cipher);
+int PEM_get_EVP_CIPHER_INFO(const char *header, EVP_CIPHER_INFO *cipher);
// PEM_do_header decrypts |*len| bytes from |data| in-place according to the
// information in |cipher|. On success, it returns one and sets |*len| to the
diff --git a/crypto/pem/pem_lib.cc b/crypto/pem/pem_lib.cc
index 69980fe..34048a3 100644
--- a/crypto/pem/pem_lib.cc
+++ b/crypto/pem/pem_lib.cc
@@ -12,6 +12,8 @@
#include <stdio.h>
#include <string.h>
+#include <string_view>
+
#include <openssl/base64.h>
#include <openssl/buf.h>
#include <openssl/des.h>
@@ -29,7 +31,7 @@
#define MIN_LENGTH 4
-static int load_iv(char **fromp, unsigned char *to, size_t num);
+static int load_iv(const char **fromp, unsigned char *to, size_t num);
static int check_pem(const char *nm, const char *name);
// PEM_proc_type appends a Proc-Type header to |buf|, determined by |type|.
@@ -143,19 +145,19 @@
return 0;
}
-static const EVP_CIPHER *cipher_by_name(const char *name) {
+static const EVP_CIPHER *cipher_by_name(std::string_view name) {
// This is similar to the (deprecated) function |EVP_get_cipherbyname|. Note
// the PEM code assumes that ciphers have at least 8 bytes of IV, at most 20
// bytes of overhead and generally behave like CBC mode.
- if (0 == strcmp(name, SN_des_cbc)) {
+ if (name == SN_des_cbc) {
return EVP_des_cbc();
- } else if (0 == strcmp(name, SN_des_ede3_cbc)) {
+ } else if (name == SN_des_ede3_cbc) {
return EVP_des_ede3_cbc();
- } else if (0 == strcmp(name, SN_aes_128_cbc)) {
+ } else if (name == SN_aes_128_cbc) {
return EVP_aes_128_cbc();
- } else if (0 == strcmp(name, SN_aes_192_cbc)) {
+ } else if (name == SN_aes_192_cbc) {
return EVP_aes_192_cbc();
- } else if (0 == strcmp(name, SN_aes_256_cbc)) {
+ } else if (name == SN_aes_256_cbc) {
return EVP_aes_256_cbc();
} else {
return NULL;
@@ -377,11 +379,7 @@
return 1;
}
-int PEM_get_EVP_CIPHER_INFO(char *header, EVP_CIPHER_INFO *cipher) {
- const EVP_CIPHER *enc = NULL;
- char *p, c;
- char **header_pp = &header;
-
+int PEM_get_EVP_CIPHER_INFO(const char *header, EVP_CIPHER_INFO *cipher) {
cipher->cipher = NULL;
OPENSSL_memset(cipher->iv, 0, sizeof(cipher->iv));
if ((header == NULL) || (*header == '\0') || (*header == '\n')) {
@@ -418,40 +416,38 @@
}
header += 10;
- p = header;
+ const char *p = header;
for (;;) {
- c = *header;
+ char c = *header;
if (!((c >= 'A' && c <= 'Z') || c == '-' || OPENSSL_isdigit(c))) {
break;
}
header++;
}
- *header = '\0';
- cipher->cipher = enc = cipher_by_name(p);
- *header = c;
+ cipher->cipher = cipher_by_name(std::string_view(p, header - p));
header++;
-
- if (enc == NULL) {
+ if (cipher->cipher == NULL) {
OPENSSL_PUT_ERROR(PEM, PEM_R_UNSUPPORTED_ENCRYPTION);
return 0;
}
// The IV parameter must be at least 8 bytes long to be used as the salt in
// the KDF. (This should not happen given |cipher_by_name|.)
- if (EVP_CIPHER_iv_length(enc) < 8) {
+ if (EVP_CIPHER_iv_length(cipher->cipher) < 8) {
assert(0);
OPENSSL_PUT_ERROR(PEM, PEM_R_UNSUPPORTED_ENCRYPTION);
return 0;
}
- if (!load_iv(header_pp, &(cipher->iv[0]), EVP_CIPHER_iv_length(enc))) {
+ const char **header_pp = &header;
+ if (!load_iv(header_pp, cipher->iv, EVP_CIPHER_iv_length(cipher->cipher))) {
return 0;
}
return 1;
}
-static int load_iv(char **fromp, unsigned char *to, size_t num) {
+static int load_iv(const char **fromp, unsigned char *to, size_t num) {
uint8_t v;
- char *from;
+ const char *from;
from = *fromp;
for (size_t i = 0; i < num; i++) {
