Merge remote-tracking branch 'origin/topic/dev/bugrpone-narrowing-conversions-base64'

* origin/topic/dev/bugrpone-narrowing-conversions-base64:
  Proof of Concept on Bugprone Narrowing Conversions: Base64

I fixed a few bugs in this while merging; Decode now signals success or
error (which did not work before). I removed the new variables in
mime.cc and just switched more to size_t - the new variables introduced
a bug because they shadowed the changes of the Decode call.

GH-616

CHANGES

@@ -1,4 +1,8 @@
 
+3.1.0-dev.177 | 2019-10-14 20:13:24 -0700
+
+  * Proof of Concept on Bugprone Narrowing Conversions: Base64 (Dev Bali, Corelight)
+
 3.1.0-dev.175 | 2019-10-12 10:39:49 -0700
 
   * Use Ubuntu 14.04 for Travis CI environment (Jon Siwek, Corelight)

VERSION

@@ -1 +1 @@
-3.1.0-dev.175
+3.1.0-dev.177

src/Base64.cc

@@ -5,9 +5,9 @@
 int Base64Converter::default_base64_table[256];
 const string Base64Converter::default_alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
 
-void Base64Converter::Encode(int len, const unsigned char* data, int* pblen, char** pbuf)
+void Base64Converter::Encode(size_t len, const unsigned char* data, size_t* pblen, char** pbuf)
 	{
-	int blen;
+	size_t blen;
 	char *buf;
 
 	if ( ! pbuf )
@@ -23,12 +23,12 @@ void Base64Converter::Encode(int len, const unsigned char* data, int* pblen, cha
 		}
 	else
 		{
-		blen = (int)(4 * ceil((double)len / 3));
+		blen = (size_t)(4 * ceil((double)len / 3));
 		*pbuf = buf = new char[blen];
 		*pblen = blen;
 		}
 
-	for ( int i = 0, j = 0; (i < len) && ( j < blen ); )
+	for ( size_t i = 0, j = 0; (i < len) && ( j < blen ); )
 		{
 			uint32_t bit32 = data[i++]  << 16;
 			bit32 += (i++ < len ? data[i-1] : 0) << 8; 
@@ -107,9 +107,9 @@ Base64Converter::~Base64Converter()
 		delete [] base64_table;
 	}
 
-int Base64Converter::Decode(int len, const char* data, int* pblen, char** pbuf)
+std::pair<size_t, bool> Base64Converter::Decode(size_t len, const char* data, size_t* pblen, char** pbuf)
 	{
-	int blen;
+	size_t blen;
 	char* buf;
 
 	// Initialization of table on first_time call of Decode.
@@ -128,11 +128,11 @@ int Base64Converter::Decode(int len, const char* data, int* pblen, char** pbuf)
 		{
 		// Estimate the maximal number of 3-byte groups needed,
 		// plus 1 byte for the optional ending NUL.
-		blen = int((len + base64_group_next + 3) / 4) * 3 + 1;
+		blen = size_t((len + base64_group_next + 3) / 4) * 3 + 1;
 		*pbuf = buf = new char[blen];
 		}
 
-	int dlen = 0;
+	size_t dlen = 0;
 
 	while ( 1 )
 		{
@@ -183,21 +183,23 @@ int Base64Converter::Decode(int len, const char* data, int* pblen, char** pbuf)
 
 		int k = base64_table[(unsigned char) data[dlen]];
 		if ( k >= 0 )
-			base64_group[base64_group_next++] = k;
+			base64_group[base64_group_next++] = static_cast<char> (k);
 		else
 			{
 			if ( ++errored == 1 )
+				{
 				IllegalEncoding(fmt("character %d ignored by Base64 decoding", (int) (data[dlen])));
 				}
+			}
 
 		++dlen;
 		}
 
 	*pblen = buf - *pbuf;
-	return dlen;
+	return {dlen, errored == 0};
 	}
 
-int Base64Converter::Done(int* pblen, char** pbuf)
+int Base64Converter::Done(size_t* pblen, char** pbuf)
 	{
 	const char* padding = "===";
 
@@ -225,12 +227,12 @@ BroString* decode_base64(const BroString* s, const BroString* a, Connection* con
 		return 0;
 		}
 
-	int buf_len = int((s->Len() + 3) / 4) * 3 + 1;
-	int rlen2, rlen = buf_len;
+	size_t buf_len = size_t((s->Len() + 3) / 4) * 3 + 1;
+	size_t rlen2, rlen = buf_len;
 	char* rbuf2, *rbuf = new char[rlen];
 
 	Base64Converter dec(conn, a ? a->CheckString() : "");
-	if ( dec.Decode(s->Len(), (const char*) s->Bytes(), &rlen, &rbuf) == -1 )
+	if ( dec.Decode(s->Len(), (const char*) s->Bytes(), &rlen, &rbuf).second == false )
 		goto err;
 
 	rlen2 = buf_len - rlen;
@@ -245,7 +247,7 @@ BroString* decode_base64(const BroString* s, const BroString* a, Connection* con
 
 err:
 	delete [] rbuf;
-	return 0;
+	return nullptr;
 	}
 
 BroString* encode_base64(const BroString* s, const BroString* a, Connection* conn)
@@ -258,7 +260,7 @@ BroString* encode_base64(const BroString* s, const BroString* a, Connection* con
 		}
 
 	char* outbuf = 0;
-	int outlen = 0;
+	size_t outlen = 0;
 	Base64Converter enc(conn, a ? a->CheckString() : "");
 	enc.Encode(s->Len(), (const unsigned char*) s->Bytes(), &outlen, &outbuf);
 

src/Base64.h

@@ -1,8 +1,10 @@
-#pragma once
+#ifndef base64_h
+#define base64_h
 
 #include <assert.h>
 #include <stdio.h>
 #include <string.h>
+#include <utility>
 
 #include "util.h"
 #include "BroString.h"
@@ -27,13 +29,12 @@ public:
 	// an appropriate size will be new'd and *buf will point
 	// to the buffer on return. *blen holds the length of
 	// decoded data on return.  The function returns the number of
-	// input bytes processed, since the decoding will stop when there
-	// is not enough output buffer space.
+	// input bytes processed and a success flag.
 
-	int Decode(int len, const char* data, int* blen, char** buf);
-	void Encode(int len, const unsigned char* data, int* blen, char** buf);
+	std::pair<size_t, bool> Decode(size_t len, const char* data, size_t* pblen, char** buf);
+	void Encode(size_t len, const unsigned char* data, size_t* blen, char** buf);
 
-	int Done(int* pblen, char** pbuf);
+	int Done(size_t* pblen, char** pbuf);
 	int HasData() const { return base64_group_next != 0; }
 
 	// True if an error has occurred.
@@ -70,3 +71,5 @@ protected:
 
 BroString* decode_base64(const BroString* s, const BroString* a = 0, Connection* conn = 0);
 BroString* encode_base64(const BroString* s, const BroString* a = 0, Connection* conn = 0);
+
+#endif /* base64_h */

src/analyzer/protocol/mime/MIME.cc

@@ -1124,14 +1124,14 @@ void MIME_Entity::DecodeQuotedPrintable(int len, const char* data)
 
 void MIME_Entity::DecodeBase64(int len, const char* data)
 	{
-	int rlen;
+	size_t rlen;
 	char rbuf[128];
 
 	while ( len > 0 )
 		{
 		rlen = 128;
 		char* prbuf = rbuf;
-		int decoded = base64_decoder->Decode(len, data, &rlen, &prbuf);
+		int decoded = base64_decoder->Decode(len, data, &rlen, &prbuf).first; // typecasting for now
 		DataOctets(rlen, rbuf);
 		len -= decoded; data += decoded;
 		}
@@ -1161,11 +1161,10 @@ void MIME_Entity::FinishDecodeBase64()
 	if ( ! base64_decoder )
 		return;
 
-	int rlen = 128;
+	size_t rlen = 128;
 	char rbuf[128];
 	char* prbuf = rbuf;
-
-	if ( base64_decoder->Done(&rlen, &prbuf) )
+	if ( base64_decoder->Done(&rlen, &prbuf) ) // typecasting for now
 		{ // some remaining data
 		if ( rlen > 0 )
 			DataOctets(rlen, rbuf);