Change X509 extension value parsing to not abort on malloc failures.

Also comes with factoring that out in to it's own function and
additional error check before using a return value from BIO_pending.

src/file_analysis/analyzer/x509/X509.cc

@@ -159,6 +159,49 @@ RecordVal* file_analysis::X509::ParseCertificate(X509Val* cert_val)
 	return pX509Cert;
 	}
 
+StringVal* file_analysis::X509::GetExtensionFromBIO(BIO* bio)
+	{
+	BIO_flush(bio);
+	ERR_clear_error();
+	int length = BIO_pending(bio);
+
+	if ( ERR_peek_error() != 0 )
+		{
+		char tmp[120];
+		ERR_error_string(ERR_get_error(), tmp);
+		reporter->Error("X509::GetExtensionFromBIO: %s", tmp);
+		BIO_free_all(bio);
+		return 0;
+		}
+
+	if ( length == 0 )
+		{
+		BIO_free_all(bio);
+		return new StringVal("");
+		}
+
+	// TODO: see about using regular malloc here, there were unknown problems
+	// using anything other than OPENSSL_malloc that need investigation.
+	char* buffer = (char*) OPENSSL_malloc(length);
+
+	if ( ! buffer )
+		{
+		// Just emit an error here and try to continue instead of aborting
+		// because it's unclear the length value is very reliable.
+		reporter->Error("X509::GetExtensionFromBIO malloc(%d) failed", length);
+		BIO_free_all(bio);
+		return 0;
+		}
+
+	BIO_read(bio, (void*) buffer, length);
+	StringVal* ext_val = new StringVal(length, buffer);
+
+	OPENSSL_free(buffer);
+	BIO_free_all(bio);
+
+	return ext_val;
+	}
+
 void file_analysis::X509::ParseExtension(X509_EXTENSION* ex)
 	{
 	char name[256];
@@ -178,20 +221,10 @@ void file_analysis::X509::ParseExtension(X509_EXTENSION* ex)
 	if( ! X509V3_EXT_print(bio, ex, 0, 0))
 		M_ASN1_OCTET_STRING_print(bio,ex->value);
 
-	BIO_flush(bio);
+	StringVal* ext_val = GetExtensionFromBIO(bio);
-	int length = BIO_pending(bio);
 
-	// Use OPENSSL_malloc here. Using new or anything else can lead
+	if ( ! ext_val )
-	// to interesting, hard to debug segfaults.
+		ext_val = new StringVal(0, "");
-	char *buffer = (char*) OPENSSL_malloc(length);
-
-	if ( ! buffer )
-		out_of_memory("X509::ParseExtension");
-
-	BIO_read(bio, (void*)buffer, length);
-	StringVal* ext_val = new StringVal(length, buffer);
-	OPENSSL_free(buffer);
-	BIO_free_all(bio);
 
 	RecordVal* pX509Ext = new RecordVal(BifType::Record::X509::Extension);
 	pX509Ext->Assign(0, new StringVal(name));

src/file_analysis/analyzer/x509/X509.h

@@ -37,6 +37,14 @@ public:
 	static file_analysis::Analyzer* Instantiate(RecordVal* args, File* file)
 		{ return new X509(args, file); }
 
+	/**
+	 * Retrieve an X509 extension value from an OpenSSL BIO to which it was
+	 * written.
+	 * @param bio the OpenSSL BIO to read.
+	 * @return The X509 extension value.
+	 */
+	static StringVal* GetExtensionFromBIO(BIO* bio);
+
 protected:
 	X509(RecordVal* args, File* file);
 

src/file_analysis/analyzer/x509/functions.bif

@@ -78,18 +78,10 @@ function x509_get_certificate_string%(cert: opaque of x509, pem: bool &default=F
 	else
 		i2d_X509_bio(bio, h->GetCertificate());
 
-	BIO_flush(bio);
+	StringVal* ext_val = file_analysis::X509::GetExtensionFromBIO(bio);
-	int length = BIO_pending(bio);
-	// use OPENSS_malloc here. Otherwhise, interesting problems will happen.
-	char *buffer = (char*) OPENSSL_malloc(length);
 
-	if ( ! buffer )
+	if ( ! ext_val )
-		out_of_memory("x509_get_certificate_string");
+		ext_val = new StringVal("");
-
-	BIO_read(bio, (void*) buffer, length);
-	StringVal* ext_val = new StringVal(length, buffer);
-	OPENSSL_free(buffer);
-	BIO_free_all(bio);
 
 	return ext_val;
 	%}