From d3f963e54c68e2f50751c090e6863b192e911c5a Mon Sep 17 00:00:00 2001 From: Tilman Sauerbeck Date: Fri, 18 Aug 2006 21:16:23 +0200 Subject: [PATCH] Sanitized vcedit_write() (and related) code some more. --- ext/vcedit.c | 285 +++++++++++++++++++++++++-------------------------- 1 file changed, 141 insertions(+), 144 deletions(-) diff --git a/ext/vcedit.c b/ext/vcedit.c index f3ad2c9..f5f493a 100644 --- a/ext/vcedit.c +++ b/ext/vcedit.c @@ -47,23 +47,47 @@ struct vcedit_state_St { bool opened; long serial; - unsigned char *mainbuf; - unsigned char *bookbuf; - int mainlen; - int booklen; + + ogg_packet packet_main; + ogg_packet packet_code_books; + char *vendor; int prevW; - int extrapage; - int eosin; + + bool extra_page; + bool eos; char filename[0]; }; +static void +ogg_packet_init (ogg_packet *p, unsigned char *buf, long len) +{ + p->packet = buf; + p->bytes = len; + + p->b_o_s = p->e_o_s = 0; + p->granulepos = p->packetno = 0; +} + +static bool +ogg_packet_dup_data (ogg_packet *p) +{ + unsigned char *tmp; + + tmp = malloc (p->bytes); + if (!tmp) + return false; + + memcpy (tmp, p->packet, p->bytes); + p->packet = tmp; + + return true; +} + static void vcedit_state_free (vcedit_state *s) { - free (s->mainbuf); - free (s->bookbuf); free (s->vendor); if (s->in) { @@ -79,6 +103,9 @@ vcedit_state_init (vcedit_state *s, const char *filename) { s->refcount = 1; + ogg_packet_init (&s->packet_main, NULL, 0); + ogg_packet_init (&s->packet_code_books, NULL, 0); + strcpy (s->filename, filename); return true; @@ -126,13 +153,8 @@ vcedit_clear_internals (vcedit_state *s) free (s->vendor); s->vendor = NULL; - free (s->mainbuf); - s->mainbuf = NULL; - s->mainlen = 0; - - free (s->bookbuf); - s->bookbuf = NULL; - s->booklen = 0; + ogg_packet_clear (&s->packet_main); + ogg_packet_clear (&s->packet_code_books); s->serial = 0; s->opened = false; @@ -166,13 +188,15 @@ _v_writestring (oggpack_buffer *o, char *s, int len) oggpack_write (o, *s++, 8); } -static int -_commentheader_out (vcedit_state *s, ogg_packet *op) +static bool +write_comments (vcedit_state *s, ogg_packet *packet) { oggpack_buffer opb; size_t len; int i; + ogg_packet_init (packet, NULL, 0); + oggpack_writeinit (&opb); /* preamble */ @@ -199,17 +223,17 @@ _commentheader_out (vcedit_state *s, ogg_packet *op) oggpack_write (&opb, 1, 1); - op->bytes = oggpack_bytes (&opb); - op->packet = _ogg_malloc (op->bytes); + packet->bytes = oggpack_bytes (&opb); - memcpy (op->packet, opb.buffer, op->bytes); + packet->packet = _ogg_malloc (packet->bytes); + if (!packet->packet) + return false; - op->b_o_s = op->e_o_s = 0; - op->granulepos = 0; + memcpy (packet->packet, opb.buffer, packet->bytes); oggpack_writeclear (&opb); - return 0; + return true; } static int @@ -237,7 +261,7 @@ _fetch_next_packet (vcedit_state *s, ogg_packet *p, ogg_page *page) if (result == 1) return 1; - if (s->eosin) + if (s->eos) return 0; while (ogg_sync_pageout (&s->oy, page) != 1) { @@ -250,10 +274,11 @@ _fetch_next_packet (vcedit_state *s, ogg_packet *p, ogg_page *page) } if (ogg_page_eos (page)) - s->eosin = 1; + s->eos = true; else if (ogg_page_serialno (page) != s->serial) { - s->eosin = 1; - s->extrapage = 1; + s->eos = true; + s->extra_page = true; + return 0; } @@ -266,9 +291,8 @@ vcedit_error vcedit_open (vcedit_state *s) { vcedit_error ret; - ogg_packet *header; - ogg_packet header_main, header_comments, header_codebooks; - ogg_page og; + ogg_packet packet_comments, *header = &packet_comments; + ogg_page page; struct stat st; char *buffer; size_t bytes, total = 0; @@ -296,34 +320,36 @@ vcedit_open (vcedit_state *s) total += bytes; ogg_sync_wrote (&s->oy, bytes); - } while (ogg_sync_pageout (&s->oy, &og) != 1); + } while (ogg_sync_pageout (&s->oy, &page) != 1); - s->serial = ogg_page_serialno (&og); + s->serial = ogg_page_serialno (&page); ogg_stream_init (&s->os, s->serial); vorbis_info_init (&s->vi); vorbis_comment_init (&s->vc); - if (ogg_stream_pagein (&s->os, &og) < 0) { + if (ogg_stream_pagein (&s->os, &page) < 0) { ret = VCEDIT_ERR_INVAL; goto err; } - if (ogg_stream_packetout (&s->os, &header_main) != 1) { + if (ogg_stream_packetout (&s->os, &s->packet_main) != 1) { ret = VCEDIT_ERR_INVAL; goto err; } - if (vorbis_synthesis_headerin (&s->vi, &s->vc, &header_main) < 0) { + if (!ogg_packet_dup_data (&s->packet_main)) { + s->packet_main.packet = NULL; ret = VCEDIT_ERR_INVAL; goto err; } - s->mainlen = header_main.bytes; - s->mainbuf = malloc (s->mainlen); - memcpy (s->mainbuf, header_main.packet, header_main.bytes); + if (vorbis_synthesis_headerin (&s->vi, &s->vc, &s->packet_main) < 0) { + ret = VCEDIT_ERR_INVAL; + goto err; + } - header = &header_comments; + ogg_packet_init (&packet_comments, NULL, 0); while (i < 2) { if (feof (s->in) || ferror (s->in)) { @@ -334,14 +360,14 @@ vcedit_open (vcedit_state *s) while (i < 2) { int result; - result = ogg_sync_pageout (&s->oy, &og); + result = ogg_sync_pageout (&s->oy, &page); if (!result) break; /* Too little data so far */ if (result != 1) continue; - ogg_stream_pagein (&s->os, &og); + ogg_stream_pagein (&s->os, &page); while (i < 2) { result = ogg_stream_packetout (&s->os, header); @@ -353,15 +379,15 @@ vcedit_open (vcedit_state *s) goto err; } - vorbis_synthesis_headerin (&s->vi, &s->vc, header); - - if (i++ == 1) { - s->booklen = header->bytes; - s->bookbuf = malloc (s->booklen); - memcpy (s->bookbuf, header->packet, header->bytes); + if (i++ == 1 && !ogg_packet_dup_data (header)) { + header->packet = NULL; + ret = VCEDIT_ERR_INVAL; + goto err; } - header = &header_codebooks; + vorbis_synthesis_headerin (&s->vi, &s->vc, header); + + header = &s->packet_code_books; } } @@ -385,7 +411,7 @@ err: return ret; } -static int +static bool write_data (const void *buf, size_t size, size_t nmemb, FILE *stream) { while (nmemb > 0) { @@ -393,26 +419,33 @@ write_data (const void *buf, size_t size, size_t nmemb, FILE *stream) w = fwrite (buf, size, nmemb, stream); if (!w && ferror (stream)) - return 0; + return false; nmemb -= w; buf += size * w; } - return 1; + return true; +} + +static bool +write_page (FILE *f, ogg_page *p) +{ + return write_data (p->header, 1, p->header_len, f) && + write_data (p->body, 1, p->body_len, f); } vcedit_error vcedit_write (vcedit_state *s) { - ogg_stream_state streamout; - ogg_packet header_main, header_comments, header_codebooks, op; - ogg_page ogout, ogin; + ogg_stream_state stream; + ogg_packet packet; + ogg_page page_out, page_in; ogg_int64_t granpos = 0; FILE *out; char *buffer, tmpfile[PATH_MAX]; - bool success = false; - int fd, result, bytes, needflush = 0, needout = 0; + bool success = false, need_flush = false, need_out = false; + int fd, result, bytes; if (!s->opened) return VCEDIT_ERR_INVAL; @@ -432,116 +465,92 @@ vcedit_write (vcedit_state *s) return VCEDIT_ERR_TMPFILE; } - s->prevW = s->extrapage = s->eosin = 0; + s->prevW = 0; + s->extra_page = s->eos = false; - header_main.bytes = s->mainlen; - header_main.packet = s->mainbuf; - header_main.b_o_s = 1; - header_main.e_o_s = 0; - header_main.granulepos = 0; + ogg_stream_init (&stream, s->serial); - header_codebooks.bytes = s->booklen; - header_codebooks.packet = s->bookbuf; - header_codebooks.b_o_s = 0; - header_codebooks.e_o_s = 0; - header_codebooks.granulepos = 0; + /* write "main" packet */ + s->packet_main.b_o_s = 1; + ogg_stream_packetin (&stream, &s->packet_main); + s->packet_main.b_o_s = 0; - ogg_stream_init (&streamout, s->serial); + /* prepare and write comments */ + if (!write_comments (s, &packet)) { + ogg_stream_clear (&stream); + unlink (tmpfile); + fclose (out); - _commentheader_out (s, &header_comments); + return VCEDIT_ERR_INVAL; + } - ogg_stream_packetin (&streamout, &header_main); - ogg_stream_packetin (&streamout, &header_comments); - ogg_stream_packetin (&streamout, &header_codebooks); + ogg_stream_packetin (&stream, &packet); + ogg_packet_clear (&packet); - while ((result = ogg_stream_flush (&streamout, &ogout))) { - if (!write_data (ogout.header, 1, ogout.header_len, out)) - goto cleanup; + /* write codebooks */ + ogg_stream_packetin (&stream, &s->packet_code_books); - if (!write_data (ogout.body, 1, ogout.body_len, out)) + while (ogg_stream_flush (&stream, &page_out)) + if (!write_page (out, &page_out)) goto cleanup; - } - while (_fetch_next_packet (s, &op, &ogin)) { + while (_fetch_next_packet (s, &packet, &page_in)) { + bool write = false; int size; - size = _blocksize (s, &op); + size = _blocksize (s, &packet); granpos += size; - if (needflush) { - if (ogg_stream_flush (&streamout, &ogout)) { - if (!write_data (ogout.header, 1, ogout.header_len, out)) - goto cleanup; - - if (!write_data (ogout.body, 1, ogout.body_len, out)) - goto cleanup; - } - } else if (needout) { - if (ogg_stream_pageout (&streamout, &ogout)) { - if (!write_data (ogout.header, 1, ogout.header_len, out)) - goto cleanup; - - if (!write_data (ogout.body, 1, ogout.body_len, out)) - goto cleanup; - } + if (need_flush) { + write = ogg_stream_flush (&stream, &page_out); + } else if (need_out) { + write = ogg_stream_pageout (&stream, &page_out); } - needflush = needout = 0; + if (write && !write_page (out, &page_out)) + goto cleanup; + + need_flush = need_out = false; - if (op.granulepos == -1) { - op.granulepos = granpos; - ogg_stream_packetin (&streamout, &op); + if (packet.granulepos == -1) { + packet.granulepos = granpos; + ogg_stream_packetin (&stream, &packet); } else { /* granulepos is set, validly. Use it, and force a flush to * account for shortened blocks (vcut) when appropriate */ - if (granpos > op.granulepos) { - granpos = op.granulepos; - ogg_stream_packetin (&streamout, &op); - needflush = 1; + if (granpos > packet.granulepos) { + granpos = packet.granulepos; + ogg_stream_packetin (&stream, &packet); + need_flush = true; } else { - ogg_stream_packetin (&streamout, &op); - needout = 1; + ogg_stream_packetin (&stream, &packet); + need_out = true; } } } - streamout.e_o_s = 1; + stream.e_o_s = 1; - while (ogg_stream_flush (&streamout, &ogout)) { - if (!write_data (ogout.header, 1, ogout.header_len, out)) + while (ogg_stream_flush (&stream, &page_out)) + if (!write_page (out, &page_out)) goto cleanup; - if (!write_data (ogout.body, 1, ogout.body_len, out)) - goto cleanup; - } - - if (s->extrapage) { - if (!write_data (ogin.header, 1, ogin.header_len, out)) - goto cleanup; - - if (!write_data (ogin.body, 1, ogin.body_len, out)) - goto cleanup; - } + if (s->extra_page && !write_page (out, &page_in)) + goto cleanup; /* clear it, because not all paths to here do */ - s->eosin = 0; + s->eos = false; do { /* We copy the rest of the stream (other logical streams) * through, a page at a time. */ - while ((result = ogg_sync_pageout (&s->oy, &ogout))) { - if (result != 1) - continue; - + while ((result = ogg_sync_pageout (&s->oy, &page_out))) { /* Don't bother going through the rest, we can just * write the page out now */ - if (!write_data (ogout.header, 1, ogout.header_len, out)) - goto cleanup; - - if (!write_data (ogout.body, 1, ogout.body_len, out)) + if (result == 1 && !write_page (out, &page_out)) goto cleanup; } @@ -551,11 +560,9 @@ vcedit_write (vcedit_state *s) if (ferror (s->in)) goto cleanup; + } while (bytes || !feof (s->in)); - s->eosin = !bytes && feof (s->in); - } while (!s->eosin); - - success = true; + s->eos = success = true; cleanup: fclose (s->in); @@ -570,19 +577,9 @@ cleanup: chmod (s->filename, s->file_mode); } - ogg_stream_clear (&streamout); - - /* We don't ogg_packet_clear() this, because the memory was - * allocated in _commentheader_out(), so we mirror that here - */ - _ogg_free (header_comments.packet); - - free (s->mainbuf); - free (s->bookbuf); - - s->mainbuf = s->bookbuf = NULL; + ogg_stream_clear (&stream); - if (!s->eosin) + if (!s->eos) return VCEDIT_ERR_INVAL; vcedit_clear_internals (s); -- 2.30.2