[Pkg-shadow-devel] [pkg-shadow-Bugs][313942] new_line gets truncated in merge_group_entries

[pkg-shadow-bugs at alioth.debian.org]

pkg-shadow-Bugs item #313942 was changed at 2013-08-04 13:02 by Nicolas François
You can respond by visiting: 
https://alioth.debian.org/tracker/?func=detail&atid=411478&aid=313942&group_id=30580

Status: Pending
Priority: 3
Submitted By: Brad Hubbard (badone-guest)
Assigned to: Nobody (None)
Summary: new_line gets truncated in merge_group_entries 
Category: None
Group: None
Resolution: None


Initial Comment:
In the following code allowance is made for the terminating NULL in new_line_len but not the newline char that is added when the two lines are concatenated. The result is new_line ends up one character short.

    314         /* Concatenate the 2 lines */
    315         new_line_len = strlen (gr1->line) + strlen (gr2->line) +1;
    316         new_line = (char *)malloc ((new_line_len + 1) * sizeof(char*));
    317         if (NULL == new_line) {
    318                 errno = ENOMEM;
    319                 return NULL;
    320         }
    321         snprintf(new_line, new_line_len, "%s\n%s", gr1->line, gr2->line);
    322         new_line[new_line_len] = '\0';

Patch attached.

----------------------------------------------------------------------

Comment By: Nicolas François (nekral)
Date: 2013-08-04 13:02

Message:
sizeof(char*) removed also. (but overall, that's what saved it from overflow ;)

Regarding the size provided to snprintf(), I do not see the problem. snprintf() will not be able to copy the terminating nul, but it is added just afterwards.

----------------------------------------------------------------------

Comment By: Christian Perrier (bubulle)
Date: 2013-07-29 08:07

Message:
I committed the original patch. Improved patch would be appreciated

----------------------------------------------------------------------

Comment By: Tomáš Mráz (tmraz-guest)
Date: 2013-01-29 13:05

Message:
The patch is not quite correct. The malloc is sizeof(char*) times more than needed. And the fix should be by changing the snprintf() to use new_line_len+1 which is the number of bytes we would allocate (if there wasn't the overallocation).


----------------------------------------------------------------------

You can respond by visiting: 
https://alioth.debian.org/tracker/?func=detail&atid=411478&aid=313942&group_id=30580