[pacman-dev] [PATCH] util: fix line length calc in _alpm_archive_fgets

[Allan McRae]

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 <dreisner at archlinux.org>
> ---
> 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;