diff --git a/ChangeLog b/ChangeLog index a919221..a2d1f55 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2012-07-17 + + * src/b64.c: Use ceil instead of floor to compute the size + of the data buffer. This prevents a one-byte heap buffer + overflow. Thanks to Justin Ferguson + for the report. + 2008-06-15: * README: Release version 3.2.0. diff --git a/src/b64.c b/src/b64.c index b8736da..b949782 100644 --- a/src/b64.c +++ b/src/b64.c @@ -235,7 +235,7 @@ int otrl_base64_otr_decode(const char *msg, unsigned char **bufp, } /* Base64-decode the message */ - rawlen = ((msglen-5) / 4) * 3; /* maximum possible */ + rawlen = ((msglen-5+3) / 4) * 3; /* maximum possible */ rawmsg = malloc(rawlen); if (!rawmsg && rawlen > 0) { return -1; diff --git a/ChangeLog b/ChangeLog index a2d1f55..bfae496 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2012-07-19 + + * src/b64.[ch], src/proto.c: Clean up the previous b64 patch + and apply it to all places where otrl_base64_decode() is called. + 2012-07-17 * src/b64.c: Use ceil instead of floor to compute the size diff --git a/src/b64.c b/src/b64.c index b949782..9e35251 100644 --- a/src/b64.c +++ b/src/b64.c @@ -55,7 +55,7 @@ VERSION HISTORY: \******************************************************************* */ /* system headers */ -#include +#include #include /* libotr headers */ @@ -147,8 +147,9 @@ static size_t decode(unsigned char *out, const char *in, size_t b64len) * base64 decode data. Skip non-base64 chars, and terminate at the * first '=', or the end of the buffer. * - * The buffer data must contain at least (base64len / 4) * 3 bytes of - * space. This function will return the number of bytes actually used. + * The buffer data must contain at least ((base64len+3) / 4) * 3 bytes + * of space. This function will return the number of bytes actually + * used. */ size_t otrl_base64_decode(unsigned char *data, const char *base64data, size_t base64len) @@ -234,13 +235,18 @@ int otrl_base64_otr_decode(const char *msg, unsigned char **bufp, return -2; } + /* Skip over the "?OTR:" */ + otrtag += 5; + msglen -= 5; + /* Base64-decode the message */ - rawlen = ((msglen-5+3) / 4) * 3; /* maximum possible */ + rawlen = OTRL_B64_MAX_DECODED_SIZE(msglen); /* maximum possible */ rawmsg = malloc(rawlen); if (!rawmsg && rawlen > 0) { return -1; } - rawlen = otrl_base64_decode(rawmsg, otrtag+5, msglen-5); /* actual size */ + + rawlen = otrl_base64_decode(rawmsg, otrtag, msglen); /* actual size */ *bufp = rawmsg; *lenp = rawlen; diff --git a/src/b64.h b/src/b64.h index 34ef03f..dd0e115 100644 --- a/src/b64.h +++ b/src/b64.h @@ -20,6 +20,19 @@ #ifndef __B64_H__ #define __B64_H__ +#include + +/* Base64 encodes blocks of this many bytes: */ +#define OTRL_B64_DECODED_LEN 3 +/* into blocks of this many bytes: */ +#define OTRL_B64_ENCODED_LEN 4 + +/* An encoded block of length encoded_len can turn into a maximum of + * this many decoded bytes: */ +#define OTRL_B64_MAX_DECODED_SIZE(encoded_len) \ + (((encoded_len + OTRL_B64_ENCODED_LEN - 1) / OTRL_B64_ENCODED_LEN) \ + * OTRL_B64_DECODED_LEN) + /* * base64 encode data. Insert no linebreaks or whitespace. * @@ -33,8 +46,9 @@ size_t otrl_base64_encode(char *base64data, const unsigned char *data, * base64 decode data. Skip non-base64 chars, and terminate at the * first '=', or the end of the buffer. * - * The buffer data must contain at least (base64len / 4) * 3 bytes of - * space. This function will return the number of bytes actually used. + * The buffer data must contain at least ((base64len+3) / 4) * 3 bytes + * of space. This function will return the number of bytes actually + * used. */ size_t otrl_base64_decode(unsigned char *data, const char *base64data, size_t base64len); diff --git a/src/proto.c b/src/proto.c index 3f8c987..0374dfe 100644 --- a/src/proto.c +++ b/src/proto.c @@ -537,13 +537,17 @@ gcry_error_t otrl_proto_data_read_flags(const char *datamsg, msglen = strlen(otrtag); } + /* Skip over the "?OTR:" */ + otrtag += 5; + msglen -= 5; + /* Base64-decode the message */ - rawlen = ((msglen-5) / 4) * 3; /* maximum possible */ + rawlen = OTRL_B64_MAX_DECODED_SIZE(msglen); /* maximum possible */ rawmsg = malloc(rawlen); if (!rawmsg && rawlen > 0) { return gcry_error(GPG_ERR_ENOMEM); } - rawlen = otrl_base64_decode(rawmsg, otrtag+5, msglen-5); /* actual size */ + rawlen = otrl_base64_decode(rawmsg, otrtag, msglen); /* actual size */ bufp = rawmsg; lenp = rawlen; @@ -606,14 +610,18 @@ gcry_error_t otrl_proto_accept_data(char **plaintextp, OtrlTLV **tlvsp, msglen = strlen(otrtag); } + /* Skip over the "?OTR:" */ + otrtag += 5; + msglen -= 5; + /* Base64-decode the message */ - rawlen = ((msglen-5) / 4) * 3; /* maximum possible */ + rawlen = OTRL_B64_MAX_DECODED_SIZE(msglen); /* maximum possible */ rawmsg = malloc(rawlen); if (!rawmsg && rawlen > 0) { err = gcry_error(GPG_ERR_ENOMEM); goto err; } - rawlen = otrl_base64_decode(rawmsg, otrtag+5, msglen-5); /* actual size */ + rawlen = otrl_base64_decode(rawmsg, otrtag, msglen); /* actual size */ bufp = rawmsg; lenp = rawlen; diff --git a/toolkit/parse.c b/toolkit/parse.c index 5f357fc..16718ca 100644 --- a/toolkit/parse.c +++ b/toolkit/parse.c @@ -64,7 +64,8 @@ static unsigned char *decode(const char *msg, size_t *lenp) { const char *header, *footer; unsigned char *raw; - + size_t rawlen; + /* Find the header */ header = strstr(msg, "?OTR:"); if (!header) return NULL; @@ -75,8 +76,10 @@ static unsigned char *decode(const char *msg, size_t *lenp) footer = strchr(header, '.'); if (!footer) footer = header + strlen(header); - raw = malloc((footer-header) / 4 * 3); - if (raw == NULL && (footer-header >= 4)) return NULL; + rawlen = OTRL_B64_MAX_DECODED_SIZE(footer-header); + + raw = malloc(rawlen); + if (raw == NULL && rawlen > 0) return NULL; *lenp = otrl_base64_decode(raw, header, footer-header); return raw;