[PATCH] libp11: Use heap instead of static stack arrays

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] libp11: Use heap instead of static stack arrays

Paul Stewart
the libp11 pkcs11_init_cert() function used stack-allocated fixed
size arrays when puling variable length attributes.  This would
cause "Buffer too small" errors with valid (but large) certificate
data.

This CL patches libp11 to query the data size and allocate a buffer
suitable for holding this data.  It does so by passing a NULL value
for data and 0 for size to the C_GetAttributeValue function, which
should return successfully and return the size of the data.

This is encapsulated in a p11_attr function, so it can be called
for multiple data types in pkcs11_init_cert.  While here, add a
null termination check for the CKA_LABEL parameter since it is
assumed to be null-terminated in callers to this function.
---
 src/libp11-int.h |  2 ++
 src/p11_attr.c   | 26 ++++++++++++++++++++++++++
 src/p11_cert.c   | 35 ++++++++++++++++++++++++-----------
 3 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/src/libp11-int.h b/src/libp11-int.h
index be3965f..ce0e296 100644
--- a/src/libp11-int.h
+++ b/src/libp11-int.h
@@ -136,6 +136,8 @@ extern int pkcs11_getattr_var(PKCS11_TOKEN *, CK_OBJECT_HANDLE,
       unsigned int, void *, size_t *);
 extern int pkcs11_getattr_bn(PKCS11_TOKEN *, CK_OBJECT_HANDLE,
      unsigned int, BIGNUM **);
+extern void *pkcs11_getattr_alloc(PKCS11_TOKEN *, CK_OBJECT_HANDLE,
+  unsigned int, size_t *);
 
 #define key_getattr(key, t, p, s) \
  pkcs11_getattr(KEY2TOKEN((key)), PRIVKEY((key))->object, (t), (p), (s))
diff --git a/src/p11_attr.c b/src/p11_attr.c
index 7db82a5..9afe7c7 100644
--- a/src/p11_attr.c
+++ b/src/p11_attr.c
@@ -102,6 +102,32 @@ pkcs11_getattr_bn(PKCS11_TOKEN * token, CK_OBJECT_HANDLE object,
  return *bn ? 0 : -1;
 }
 
+void *
+pkcs11_getattr_alloc(PKCS11_TOKEN * token, CK_OBJECT_HANDLE object,
+     unsigned int type, size_t *size_out)
+{
+ size_t size = 0;
+ void *data = NULL;
+
+ if (pkcs11_getattr_var(token, object, type, NULL, &size))
+ return NULL;
+
+ data = malloc(size);
+ if (data == NULL)
+ return NULL;
+
+ memset(data, 0, size);
+ if (pkcs11_getattr_var(token, object, type, data, &size)) {
+ free(data);
+ return NULL;
+ }
+
+ if (size_out != NULL)
+ *size_out = size;
+
+ return data;
+}
+
 /*
  * Add attributes to template
  */
diff --git a/src/p11_cert.c b/src/p11_cert.c
index 3dcde82..5df87e0 100644
--- a/src/p11_cert.c
+++ b/src/p11_cert.c
@@ -136,10 +136,9 @@ static int pkcs11_init_cert(PKCS11_CTX * ctx, PKCS11_TOKEN * token,
  PKCS11_TOKEN_private *tpriv;
  PKCS11_CERT_private *kpriv;
  PKCS11_CERT *cert, *tmp;
- char label[256], data[2048];
- unsigned char id[256];
  CK_CERTIFICATE_TYPE cert_type;
  size_t size;
+ void *data;
 
  (void)ctx;
  (void)session;
@@ -168,18 +167,32 @@ static int pkcs11_init_cert(PKCS11_CTX * ctx, PKCS11_TOKEN * token,
  kpriv->object = obj;
  kpriv->parent = token;
 
- if (!pkcs11_getattr_s(token, obj, CKA_LABEL, label, sizeof(label)))
- cert->label = BUF_strdup(label);
- size = sizeof(data);
- if (!pkcs11_getattr_var(token, obj, CKA_VALUE, data, &size)) {
- const unsigned char *p = (unsigned char *) data;
+ data = pkcs11_getattr_alloc(token, obj, CKA_LABEL, &size);
+ if (data != NULL) {
+ char *label = data;
+ /* Fix any null-termination issues with the label */
+ if (label[size - 1] != '\0') {
+ label = realloc(label, size + 1);
+ if (label == NULL) {
+ free(data);
+ return -1;
+ }
+ label[size] = '\0';
+ }
+ cert->label = label;
+ }
 
+ data = pkcs11_getattr_alloc(token, obj, CKA_VALUE, &size);
+ if (data != NULL) {
+ const unsigned char *p = data;
  cert->x509 = d2i_X509(NULL, &p, size);
+ free(data);
  }
- cert->id_len = sizeof(id);
- if (!pkcs11_getattr_var(token, obj, CKA_ID, id, &cert->id_len)) {
- cert->id = (unsigned char *) malloc(cert->id_len);
- memcpy(cert->id, id, cert->id_len);
+ data = pkcs11_getattr_alloc(token, obj, CKA_ID, &cert->id_len);
+ if (data != NULL) {
+ cert->id = data;
+ } else {
+ cert->id_len = 0;
  }
 
  /* Initialize internal information */
--
2.1.0.rc2.206.gedb03e5


------------------------------------------------------------------------------
Slashdot TV.  Videos for Nerds.  Stuff that Matters.
http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
_______________________________________________
Opensc-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/opensc-devel