diff --git a/CHANGES b/CHANGES index ba487e115c..493022a847 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,26 @@ +2.4-108 | 2015-08-30 20:14:31 -0700 + + * Update Base64 decoding. (Jan Grashoefer) + + - A new built-in function, decode_base64_conn() for Base64 + decoding. It works like decode_base64() but receives an + additional connection argument that will be used for + reporting decoding errors into weird.log (instead of + reporter.log). + + - FTP, POP3, and HTTP analyzers now likewise log Base64 + decoding errors to weird.log. + + - The built-in functions decode_base64_custom() and + encode_base64_custom() are now deprecated. Their + functionality is provided directly by decode_base64() and + encode_base64(), which take an optional parameter to change + the Base64 alphabet. + + * Fix potential crash if TCP header was captured incompletely. + (Robin Sommer) + 2.4-103 | 2015-08-29 10:51:55 -0700 * Make ASN.1 date/time parsing more robust. (Johanna Amann) diff --git a/NEWS b/NEWS index 3b9efd1912..7822b15455 100644 --- a/NEWS +++ b/NEWS @@ -28,11 +28,25 @@ New Functionality information. Use with care, generating events per packet is expensive. +- A new built-in function, decode_base64_conn() for Base64 decoding. + It works like decode_base64() but receives an additional connection + argument that will be used for decoding errors into weird.log + (instead of reporter.log). + - New Bro plugins in aux/plugins: - pf_ring: Native PF_RING support. - redis: An experimental log writer for Redis. +Deprecated Functionality +------------------------ + + - The built-in functions decode_base64_custom() and + encode_base64_custom() are no longer needed and will be removed + in the future. Their functionality is now provided directly by + decode_base64() and encode_base64(), which take an optional + parameter to change the Base64 alphabet. + Bro 2.4 ======= diff --git a/VERSION b/VERSION index 29307c8aaa..f11a86a9b2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.4-103 +2.4-108 diff --git a/scripts/base/protocols/http/main.bro b/scripts/base/protocols/http/main.bro index 916723ebcb..e74ffaa8d4 100644 --- a/scripts/base/protocols/http/main.bro +++ b/scripts/base/protocols/http/main.bro @@ -270,7 +270,7 @@ event http_header(c: connection, is_orig: bool, name: string, value: string) &pr { if ( /^[bB][aA][sS][iI][cC] / in value ) { - local userpass = decode_base64(sub(value, /[bB][aA][sS][iI][cC][[:blank:]]/, "")); + local userpass = decode_base64_conn(c$id, sub(value, /[bB][aA][sS][iI][cC][[:blank:]]/, "")); local up = split_string(userpass, /:/); if ( |up| >= 2 ) { diff --git a/src/Base64.cc b/src/Base64.cc index 2ff858cad5..3644740c7e 100644 --- a/src/Base64.cc +++ b/src/Base64.cc @@ -82,7 +82,7 @@ int* Base64Converter::InitBase64Table(const string& alphabet) return base64_table; } -Base64Converter::Base64Converter(analyzer::Analyzer* arg_analyzer, const string& arg_alphabet) +Base64Converter::Base64Converter(Connection* arg_conn, const string& arg_alphabet) { if ( arg_alphabet.size() > 0 ) { @@ -98,7 +98,7 @@ Base64Converter::Base64Converter(analyzer::Analyzer* arg_analyzer, const string& base64_group_next = 0; base64_padding = base64_after_padding = 0; errored = 0; - analyzer = arg_analyzer; + conn = arg_conn; } Base64Converter::~Base64Converter() @@ -216,9 +216,9 @@ int Base64Converter::Done(int* pblen, char** pbuf) } -BroString* decode_base64(const BroString* s, const BroString* a) +BroString* decode_base64(const BroString* s, const BroString* a, Connection* conn) { - if ( a && a->Len() != 64 ) + if ( a && a->Len() != 0 && a->Len() != 64 ) { reporter->Error("base64 decoding alphabet is not 64 characters: %s", a->CheckString()); @@ -229,7 +229,7 @@ BroString* decode_base64(const BroString* s, const BroString* a) int rlen2, rlen = buf_len; char* rbuf2, *rbuf = new char[rlen]; - Base64Converter dec(0, a ? a->CheckString() : ""); + Base64Converter dec(conn, a ? a->CheckString() : ""); if ( dec.Decode(s->Len(), (const char*) s->Bytes(), &rlen, &rbuf) == -1 ) goto err; @@ -248,9 +248,9 @@ err: return 0; } -BroString* encode_base64(const BroString* s, const BroString* a) +BroString* encode_base64(const BroString* s, const BroString* a, Connection* conn) { - if ( a && a->Len() != 64 ) + if ( a && a->Len() != 0 && a->Len() != 64 ) { reporter->Error("base64 alphabet is not 64 characters: %s", a->CheckString()); @@ -259,7 +259,7 @@ BroString* encode_base64(const BroString* s, const BroString* a) char* outbuf = 0; int outlen = 0; - Base64Converter enc(0, a ? a->CheckString() : ""); + Base64Converter enc(conn, a ? a->CheckString() : ""); enc.Encode(s->Len(), (const unsigned char*) s->Bytes(), &outlen, &outbuf); return new BroString(1, (u_char*)outbuf, outlen); diff --git a/src/Base64.h b/src/Base64.h index d7e4384ac5..fb030915ef 100644 --- a/src/Base64.h +++ b/src/Base64.h @@ -8,15 +8,17 @@ #include "util.h" #include "BroString.h" #include "Reporter.h" -#include "analyzer/Analyzer.h" +#include "Conn.h" // Maybe we should have a base class for generic decoders? class Base64Converter { public: - // is used for error reporting, and it should be zero when - // the decoder is called by the built-in function decode_base64() or encode_base64(). - // Empty alphabet indicates the default base64 alphabet. - Base64Converter(analyzer::Analyzer* analyzer, const string& alphabet = ""); + // is used for error reporting. If it is set to zero (as, + // e.g., done by the built-in functions decode_base64() and + // encode_base64()), encoding-errors will go to Reporter instead of + // Weird. Usage errors go to Reporter in any case. Empty alphabet + // indicates the default base64 alphabet. + Base64Converter(Connection* conn, const string& alphabet = ""); ~Base64Converter(); // A note on Decode(): @@ -42,8 +44,8 @@ public: void IllegalEncoding(const char* msg) { // strncpy(error_msg, msg, sizeof(error_msg)); - if ( analyzer ) - analyzer->Weird("base64_illegal_encoding", msg); + if ( conn ) + conn->Weird("base64_illegal_encoding", msg); else reporter->Error("%s", msg); } @@ -63,11 +65,11 @@ protected: int base64_after_padding; int* base64_table; int errored; // if true, we encountered an error - skip further processing - analyzer::Analyzer* analyzer; + Connection* conn; }; -BroString* decode_base64(const BroString* s, const BroString* a = 0); -BroString* encode_base64(const BroString* s, const BroString* a = 0); +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 */ diff --git a/src/analyzer/protocol/ftp/FTP.cc b/src/analyzer/protocol/ftp/FTP.cc index fd38ee8f29..70d1be5777 100644 --- a/src/analyzer/protocol/ftp/FTP.cc +++ b/src/analyzer/protocol/ftp/FTP.cc @@ -206,7 +206,7 @@ void FTP_ADAT_Analyzer::DeliverStream(int len, const u_char* data, bool orig) { line = skip_whitespace(line + cmd_len, end_of_line); StringVal encoded(end_of_line - line, line); - decoded_adat = decode_base64(encoded.AsString()); + decoded_adat = decode_base64(encoded.AsString(), 0, Conn()); if ( first_token ) { @@ -273,7 +273,7 @@ void FTP_ADAT_Analyzer::DeliverStream(int len, const u_char* data, bool orig) { line += 5; StringVal encoded(end_of_line - line, line); - decoded_adat = decode_base64(encoded.AsString()); + decoded_adat = decode_base64(encoded.AsString(), 0, Conn()); } break; diff --git a/src/analyzer/protocol/mime/MIME.cc b/src/analyzer/protocol/mime/MIME.cc index 5ae8508922..d968be09cf 100644 --- a/src/analyzer/protocol/mime/MIME.cc +++ b/src/analyzer/protocol/mime/MIME.cc @@ -1134,7 +1134,15 @@ void MIME_Entity::StartDecodeBase64() delete base64_decoder; } - base64_decoder = new Base64Converter(message->GetAnalyzer()); + analyzer::Analyzer* analyzer = message->GetAnalyzer(); + + if ( ! analyzer ) + { + reporter->InternalWarning("no analyzer associated with MIME message"); + return; + } + + base64_decoder = new Base64Converter(analyzer->Conn()); } void MIME_Entity::FinishDecodeBase64() diff --git a/src/analyzer/protocol/pop3/POP3.cc b/src/analyzer/protocol/pop3/POP3.cc index 159675a8b9..b7d6aa0dcb 100644 --- a/src/analyzer/protocol/pop3/POP3.cc +++ b/src/analyzer/protocol/pop3/POP3.cc @@ -137,7 +137,7 @@ void POP3_Analyzer::ProcessRequest(int length, const char* line) ++authLines; BroString encoded(line); - BroString* decoded = decode_base64(&encoded); + BroString* decoded = decode_base64(&encoded, 0, Conn()); if ( ! decoded ) { diff --git a/src/bro.bif b/src/bro.bif index 629abe7735..04394434b3 100644 --- a/src/bro.bif +++ b/src/bro.bif @@ -2723,14 +2723,18 @@ function hexstr_to_bytestring%(hexstr: string%): string ## Encodes a Base64-encoded string. ## -## s: The string to encode +## s: The string to encode. +## +## a: An optional custom alphabet. The empty string indicates the default alphabet. +## If given, the length of *a* must be 64. For example, a custom alphabet could be +## ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``. ## ## Returns: The encoded version of *s*. ## -## .. bro:see:: encode_base64_custom decode_base64 -function encode_base64%(s: string%): string +## .. bro:see:: decode_base64 decode_base64_conn +function encode_base64%(s: string, a: string &default=""%): string %{ - BroString* t = encode_base64(s->AsString()); + BroString* t = encode_base64(s->AsString(), a->AsString()); if ( t ) return new StringVal(t); else @@ -2740,18 +2744,19 @@ function encode_base64%(s: string%): string } %} + ## Encodes a Base64-encoded string with a custom alphabet. ## -## s: The string to encode +## s: The string to encode. ## -## a: The custom alphabet. The empty string indicates the default alphabet. The -## length of *a* must be 64. For example, a custom alphabet could be +## a: An optional custom alphabet. The empty string indicates the default alphabet. +## If given, the length of *a* must be 64. For example, a custom alphabet could be ## ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``. ## ## Returns: The encoded version of *s*. ## -## .. bro:see:: encode_base64 decode_base64_custom -function encode_base64_custom%(s: string, a: string%): string +## .. bro:see:: encode_base64 decode_base64 decode_base64_conn +function encode_base64_custom%(s: string, a: string%): string &deprecated %{ BroString* t = encode_base64(s->AsString(), a->AsString()); if ( t ) @@ -2767,12 +2772,50 @@ function encode_base64_custom%(s: string, a: string%): string ## ## s: The Base64-encoded string. ## +## a: An optional custom alphabet. The empty string indicates the default alphabet. +## If given, the length of *a* must be 64. For example, a custom alphabet could be +## ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``. +## ## Returns: The decoded version of *s*. ## -## .. bro:see:: decode_base64_custom encode_base64 -function decode_base64%(s: string%): string +## .. bro:see:: decode_base64_intern encode_base64 +function decode_base64%(s: string, a: string &default=""%): string %{ - BroString* t = decode_base64(s->AsString()); + BroString* t = decode_base64(s->AsString(), a->AsString()); + if ( t ) + return new StringVal(t); + else + { + reporter->Error("error in decoding string %s", s->CheckString()); + return new StringVal(""); + } + %} + +## Decodes a Base64-encoded string that was derived from processing a connection. +## If an error is encountered decoding the string, that will be logged to +## ``weird.log`` with the associated connection, +## +## cid: The identifier of the connection that the encoding originates from. +## +## s: The Base64-encoded string. +## +## a: An optional custom alphabet. The empty string indicates the default alphabet. +## If given, the length of *a* must be 64. For example, a custom alphabet could be +## ``"!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"``. +## +## Returns: The decoded version of *s*. +## +## .. bro:see:: decode_base64 encode_base64_intern +function decode_base64_conn%(cid: conn_id, s: string, a: string &default=""%): string + %{ + Connection* conn = sessions->FindConnection(cid); + if ( ! conn ) + { + builtin_error("connection ID not a known connection", cid); + return new StringVal(""); + } + + BroString* t = decode_base64(s->AsString(), a->AsString(), conn); if ( t ) return new StringVal(t); else @@ -2792,8 +2835,8 @@ function decode_base64%(s: string%): string ## ## Returns: The decoded version of *s*. ## -## .. bro:see:: decode_base64 encode_base64_custom -function decode_base64_custom%(s: string, a: string%): string +## .. bro:see:: decode_base64 decode_base64_conn encode_base64 +function decode_base64_custom%(s: string, a: string%): string &deprecated %{ BroString* t = decode_base64(s->AsString(), a->AsString()); if ( t ) diff --git a/testing/btest/Baseline/bifs.decode_base64/out b/testing/btest/Baseline/bifs.decode_base64/out index af0d32fbb8..aa265d2148 100644 --- a/testing/btest/Baseline/bifs.decode_base64/out +++ b/testing/btest/Baseline/bifs.decode_base64/out @@ -4,3 +4,11 @@ bro bro bro bro +bro +bro +bro +bro +bro +bro +bro +bro diff --git a/testing/btest/Baseline/bifs.decode_base64_conn/weird.log b/testing/btest/Baseline/bifs.decode_base64_conn/weird.log new file mode 100644 index 0000000000..e263a05ccc --- /dev/null +++ b/testing/btest/Baseline/bifs.decode_base64_conn/weird.log @@ -0,0 +1,12 @@ +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path weird +#open 2015-08-31-03-09-20 +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer +#types time string addr port addr port string string bool string +1254722767.875996 CjhGID4nQcgTWjvg4c 10.10.1.4 1470 74.53.140.153 25 base64_illegal_encoding incomplete base64 group, padding with 12 bits of 0 F bro +1437831787.861602 CPbrpk1qSsw6ESzHV4 192.168.133.100 49648 192.168.133.102 25 base64_illegal_encoding incomplete base64 group, padding with 12 bits of 0 F bro +1437831799.610433 C7XEbhP654jzLoe3a 192.168.133.100 49655 17.167.150.73 443 base64_illegal_encoding incomplete base64 group, padding with 12 bits of 0 F bro +#close 2015-08-31-03-09-20 diff --git a/testing/btest/Baseline/bifs.encode_base64/out b/testing/btest/Baseline/bifs.encode_base64/out index 84c2c98264..3008115853 100644 --- a/testing/btest/Baseline/bifs.encode_base64/out +++ b/testing/btest/Baseline/bifs.encode_base64/out @@ -1,5 +1,9 @@ YnJv YnJv +YnJv +}n-v +YnJv +YnJv }n-v cGFkZGluZw== cGFkZGluZzE= diff --git a/testing/btest/bifs/decode_base64.bro b/testing/btest/bifs/decode_base64.bro index d4cbd2f37d..2d552a2523 100644 --- a/testing/btest/bifs/decode_base64.bro +++ b/testing/btest/bifs/decode_base64.bro @@ -6,9 +6,17 @@ global default_alphabet: string = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrs global my_alphabet: string = "!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"; print decode_base64("YnJv"); +print decode_base64("YnJv", default_alphabet); +print decode_base64("YnJv", ""); # should use default alpabet +print decode_base64("}n-v", my_alphabet); print decode_base64_custom("YnJv", default_alphabet); +print decode_base64_custom("YnJv", ""); # should use default alpabet print decode_base64_custom("}n-v", my_alphabet); print decode_base64("YnJv"); +print decode_base64("YnJv", default_alphabet); +print decode_base64("YnJv", ""); # should use default alpabet +print decode_base64("}n-v", my_alphabet); print decode_base64_custom("YnJv", default_alphabet); +print decode_base64_custom("YnJv", ""); # should use default alpabet print decode_base64_custom("}n-v", my_alphabet); diff --git a/testing/btest/bifs/decode_base64_conn.bro b/testing/btest/bifs/decode_base64_conn.bro new file mode 100644 index 0000000000..e515ed68ac --- /dev/null +++ b/testing/btest/bifs/decode_base64_conn.bro @@ -0,0 +1,8 @@ +# @TEST-EXEC: bro -r $TRACES/smtp.trace %INPUT >out +# @TEST-EXEC: btest-diff weird.log + +event connection_established(c: connection) + { + # This should be logged into weird. + print decode_base64_conn(c$id, "kaputt"); + } diff --git a/testing/btest/bifs/encode_base64.bro b/testing/btest/bifs/encode_base64.bro index a351392bb5..bbad715ecc 100644 --- a/testing/btest/bifs/encode_base64.bro +++ b/testing/btest/bifs/encode_base64.bro @@ -6,7 +6,12 @@ global default_alphabet: string = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrs global my_alphabet: string = "!#$%&/(),-.:;<>@[]^ `_{|}~abcdefghijklmnopqrstuvwxyz0123456789+?"; print encode_base64("bro"); +print encode_base64("bro", default_alphabet); +print encode_base64("bro", ""); # should use default alpabet +print encode_base64("bro", my_alphabet); + print encode_base64_custom("bro", default_alphabet); +print encode_base64_custom("bro", ""); # should use default alpabet print encode_base64_custom("bro", my_alphabet); print encode_base64("padding");