diff --git a/src/opus.cc b/src/opus.cc index 9d0315d..067cc21 100644 --- a/src/opus.cc +++ b/src/opus.cc @@ -34,34 +34,41 @@ #define le32toh(x) OSSwapLittleToHostInt32(x) #endif -int ot::parse_tags(const char *data, long len, opus_tags *tags) +ot::parse_result ot::parse_tags(const char *data, long len, opus_tags *tags) { long pos; - if (len < 8+4+4) - return -1; - if (strncmp(data, "OpusTags", 8) != 0) - return -1; + if (8 > len) + return parse_result::overflowing_magic_number; + if (memcmp(data, "OpusTags", 8) != 0) + return parse_result::bad_magic_number; // Vendor pos = 8; - tags->vendor = ot::string_view(data + pos + 4, le32toh(*((uint32_t*) (data + pos)))); - pos += 4 + tags->vendor.size(); if (pos + 4 > len) - return -1; + return parse_result::overflowing_vendor_length; + size_t vendor_length = le32toh(*((uint32_t*) (data + pos))); + if (pos + 4 + vendor_length > len) + return parse_result::overflowing_vendor_data; + tags->vendor = ot::string_view(data + pos + 4, vendor_length); + pos += 4 + tags->vendor.size(); // Count + if (pos + 4 > len) + return parse_result::overflowing_comment_count; uint32_t count = le32toh(*((uint32_t*) (data + pos))); pos += 4; // Comments for (uint32_t i = 0; i < count; ++i) { + if (pos + 4 > len) + return parse_result::overflowing_comment_length; uint32_t comment_length = le32toh(*((uint32_t*) (data + pos))); + if (pos + 4 + comment_length > len) + return parse_result::overflowing_comment_data; const char *comment_value = data + pos + 4; tags->comments.emplace_back(comment_value, comment_length); pos += 4 + comment_length; - if (pos > len) - return -1; } // Extra data tags->extra_data = ot::string_view(data + pos, static_cast(len - pos)); - return 0; + return parse_result::ok; } int ot::render_tags(opus_tags *tags, ogg_packet *op) diff --git a/src/opustags.cc b/src/opustags.cc index 3ca5bb4..3865e9f 100644 --- a/src/opustags.cc +++ b/src/opustags.cc @@ -256,7 +256,7 @@ int main(int argc, char **argv){ } } else if(packet_count == 2){ // Comment header - if(ot::parse_tags((char*) op.packet, op.bytes, &tags) == -1){ + if(ot::parse_tags((char*) op.packet, op.bytes, &tags) != ot::parse_result::ok){ error = "opustags: invalid comment header"; break; } diff --git a/src/opustags.h b/src/opustags.h index ced1791..847cad3 100644 --- a/src/opustags.h +++ b/src/opustags.h @@ -83,7 +83,18 @@ struct opus_tags { string_view extra_data; }; -int parse_tags(const char *data, long len, opus_tags *tags); +enum class parse_result { + ok = 0, + bad_magic_number = -100, + overflowing_magic_number, + overflowing_vendor_length, + overflowing_vendor_data, + overflowing_comment_count, + overflowing_comment_length, + overflowing_comment_data, +}; + +parse_result parse_tags(const char *data, long len, opus_tags *tags); int render_tags(opus_tags *tags, ogg_packet *op); void delete_tags(opus_tags *tags, const char *field); diff --git a/t/unit.cc b/t/unit.cc index adc8161..6f18d89 100644 --- a/t/unit.cc +++ b/t/unit.cc @@ -39,9 +39,9 @@ static const char standard_OpusTags[] = static bool parse_standard() { ot::opus_tags tags; - int rc = ot::parse_tags(standard_OpusTags, sizeof(standard_OpusTags) - 1, &tags); - if (rc != 0) - throw failure("ot::parse_tags did not return 0"); + auto rc = ot::parse_tags(standard_OpusTags, sizeof(standard_OpusTags) - 1, &tags); + if (rc != ot::parse_result::ok) + throw failure("ot::parse_tags did not return ok"); if (tags.vendor != ot::string_view( "opustags test packet")) throw failure("bad vendor string"); if (tags.comments.size() != 2) @@ -60,12 +60,11 @@ static bool parse_standard() static bool recode_standard() { ot::opus_tags tags; - int rc = ot::parse_tags(standard_OpusTags, sizeof(standard_OpusTags) - 1, &tags); - if (rc != 0) - throw failure("ot::parse_tags did not return 0"); + auto rc = ot::parse_tags(standard_OpusTags, sizeof(standard_OpusTags) - 1, &tags); + if (rc != ot::parse_result::ok) + throw failure("ot::parse_tags did not return ok"); ogg_packet packet; - rc = ot::render_tags(&tags, &packet); - if (rc != 0) + if (ot::render_tags(&tags, &packet) != 0) throw failure("ot::render_tags did not return 0"); if (packet.b_o_s != 0) throw failure("b_o_s should not be set"); @@ -89,14 +88,15 @@ static bool recode_padding() std::string padded_OpusTags(standard_OpusTags, sizeof(standard_OpusTags)); // ^ note: padded_OpusTags ends with a null byte here padded_OpusTags += "hello"; - int rc = ot::parse_tags(padded_OpusTags.data(), padded_OpusTags.size(), &tags); - if (rc != 0) - throw failure("ot::parse_tags did not return 0"); + auto rc = ot::parse_tags(padded_OpusTags.data(), padded_OpusTags.size(), &tags); + if (rc != ot::parse_result::ok) + throw failure("ot::parse_tags did not return ok"); if (tags.extra_data != ot::string_view("\0hello", 6)) throw failure("corrupted extra data"); // recode the packet and ensure it's exactly the same ogg_packet packet; - rc = ot::render_tags(&tags, &packet); + if (ot::render_tags(&tags, &packet) != 0) + throw failure("ot::render_tags did not return 0"); if (packet.bytes < padded_OpusTags.size()) throw failure("the packet was truncated"); if (packet.bytes > padded_OpusTags.size())