On 20/07/12 00:54, Dave Reisner wrote:
> 74274b5dc347ba70 which added the real_line_size to the buffer struct
> didn't properly account for what happens when archive_fgets has to loop
> more than once to find the end of a line. In most cases, this isn't a
> problem, but could potentially cause a longer line such as PGP signature
> to be improperly read.
>
> This patch fixes the oversight and focuses on only calculating the line
> length when we hit the end of line marker. The effective length is then
> calculated via pointer arithmetic as:
>
> (start_of_last_read + read_length) - start_of_line
>
> Signed-off-by: Dave Reisner <[email protected]>
> ---
> Allan noticed that PHP was reporting an invalid sig and bisected it back to
> me,
> (of course). This was lucky enough to only affect about .1% of the sync DB,
> and
> mostly the PGPSIG field, naturally due to it being the largest field in the DB
> by a wide margin.
>
> lib/libalpm/util.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index c2b5d44..721125a 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -1011,15 +1011,16 @@ int _alpm_archive_fgets(struct archive *a, struct
> archive_read_buffer *b)
> }
>
> if(eol) {
> - size_t len = b->real_line_size = (size_t)(eol -
> b->block_offset);
> + size_t len = (size_t)(eol - b->block_offset);
> memcpy(b->line_offset, b->block_offset, len);
> b->line_offset[len] = '\0';
> b->block_offset = eol + 1;
> + b->real_line_size = b->line_offset + len - b->line;
> /* this is the main return point; from here you can
> read b->line */
> return ARCHIVE_OK;
> } else {
> /* we've looked through the whole block but no newline,
> copy it */
> - size_t len = b->real_line_size = (size_t)(b->block +
> b->block_size - b->block_offset);
> + size_t len = (size_t)(b->block + b->block_size -
> b->block_offset);
> memcpy(b->line_offset, b->block_offset, len);
> b->line_offset += len;
> b->block_offset = b->block + b->block_size;
Just for the record... here is my version which does not break pactests:
diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
index 0db9e87..e91460e 100644
--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -947,6 +947,7 @@ int _alpm_archive_fgets(struct archive *a, struct
archive_read_buffer *b)
{
/* ensure we start populating our line buffer at the beginning */
b->line_offset = b->line;
+ b->real_line_size = 0;
while(1) {
size_t block_remaining;
@@ -1008,7 +1009,8 @@ int _alpm_archive_fgets(struct archive *a, struct
archive_read_buffer *b)
}
if(eol) {
- size_t len = b->real_line_size = (size_t)(eol -
b->block_offset);
+ size_t len = (size_t)(eol - b->block_offset);
+ b->real_line_size += len;
memcpy(b->line_offset, b->block_offset, len);
b->line_offset[len] = '\0';
b->block_offset = eol + 1;
@@ -1016,7 +1018,8 @@ int _alpm_archive_fgets(struct archive *a, struct
archive_read_buffer *b)
return ARCHIVE_OK;
} else {
/* we've looked through the whole block but no newline,
copy it */
- size_t len = b->real_line_size = (size_t)(b->block +
b->block_size -
b->block_offset);
+ size_t len = (size_t)(b->block + b->block_size -
b->block_offset);
+ b->real_line_size += len;
memcpy(b->line_offset, b->block_offset, len);
b->line_offset += len;
b->block_offset = b->block + b->block_size;
