FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 07-19-2012, 02:54 PM
Dave Reisner
 
Default util: fix line length calc in _alpm_archive_fgets

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@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] = '';
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;
--
1.7.11.2
 
Old 07-20-2012, 12:44 AM
Allan McRae
 
Default util: fix line length calc in _alpm_archive_fgets

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@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] = '';
> 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] = '';
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;
 
Old 07-20-2012, 12:47 AM
Dave Reisner
 
Default util: fix line length calc in _alpm_archive_fgets

Not necessary If you zero out the line size in the !eol case when len == 0
On Jul 19, 2012 8:44 PM, "Allan McRae" <allan@archlinux.org> wrote:

> 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@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] = '';
> > 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] = '';
> 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;
>
>
>
>
>
>
 
Old 07-20-2012, 12:55 AM
Allan McRae
 
Default util: fix line length calc in _alpm_archive_fgets

On 20/07/12 10:47, Dave Reisner wrote:
> Not necessary If you zero out the line size in the !eol case when len == 0

Ah... adding:

--- a/lib/libalpm/util.c
+++ b/lib/libalpm/util.c
@@ -1025,6 +1025,7 @@ int _alpm_archive_fgets(struct archive *a, struct
archive_
* returned on next call */
if(len == 0) {
b->line_offset[0] = '';
+ b->real_line_size = 0;
return ARCHIVE_OK;
}
}


all is good.
 
Old 07-20-2012, 03:16 AM
Dave Reisner
 
Default util: fix line length calc in _alpm_archive_fgets

Yes I mentioned this in IRC :P
On Jul 19, 2012 8:55 PM, "Allan McRae" <allan@archlinux.org> wrote:

> On 20/07/12 10:47, Dave Reisner wrote:
> > Not necessary If you zero out the line size in the !eol case when len ==
> 0
>
> Ah... adding:
>
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -1025,6 +1025,7 @@ int _alpm_archive_fgets(struct archive *a, struct
> archive_
> * returned on next call */
> if(len == 0) {
> b->line_offset[0] = '';
> + b->real_line_size = 0;
> return ARCHIVE_OK;
> }
> }
>
>
> all is good.
>
>
>
 

Thread Tools




All times are GMT. The time now is 06:33 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org