Hardcoded buffer size in gksu-run-helper

A reminder: This bug still exists in version 2.0.5-1ubuntu6. And it still prevents
update-manager from working correctly.

Thanks, I'm looking into this right now.

Could you please attach the output of:
$ gksu --debug id

What environment do you use? I would like to be able to reproduce the problem if possible to prepare a SRU (StableReleaseUpdate) for the fix.

For now I just increased the buffer to 1024 byte, but once I get instructions how to reproduce, I would like to switch it to dynamic buffer allocation.

Cheers,
 michael

This bug was fixed in the package libgksu - 2.0.5-1ubuntu8

---------------
libgksu (2.0.5-1ubuntu8) intrepid; urgency=low

  * debian/patches/21_increase_gksu_helper_buf.patch:
    - increase the buffer in gksu-run-helper to fix breakage for
      very long sn_ids (LP: #173757)

 -- Michael Vogt <email address hidden> Mon, 09 Jun 2008 16:56:40 +0200

I uploaded a simple fix (just increasing the buffer) for this into my PPA (debdiff attached) at:

deb http://ppa.launchpad.net/mvo hardy main

Please test and give me feedback, I would like to issue a SRU for this.

Thanks,
 michael

Hi.

I answered your jabber message briefly, but for completeness let me answer here as well.

gksu --debug id

gives me

STARTUP_ID: gksu/id/6533-0-chiyo_TIME1808071

And

gksu --debug echo blarg blarg blarg

gives me

STARTUP_ID: gksu/echo 'blarg' 'blarg' 'blarg'/6547-0-chiyo_TIME1874655

To get a longer startup id, it's just a question of adding more "blarg" arguments to echo, so this should
be sufficient to reproduce the problem (albeit in a constructed form).

Hm, better yet, here is reproduction recipe that shows the actual problem of xauth breaking:

yes :0 | head -30 | xargs gksu --debug xauth list

works, but

yes :0 | head -300 | xargs gksu --debug xauth list

does not.

Btw, I think you made a mistake in your proposed patch in that it now says "gchar *buffer[".
Unless your architecture of choice has single-byte pointers, I expect that will spell an
incorrect value of "buffer + 10" in the places where it's used.

Other than that it looks ok as a stopgap fix. I've been running a locally modified version of
the package with a 511 byte buffer for quite some time without encountering any concrete
problems.

Thanks for your review of the patch (and thanks for spotting the mistake in the char buffer).

Attached is a new patch, could you please review it and give it a go?

Thanks,
 MIchael

I can only find one small error in your new patch:

If fgets can not read at least one character (because of EOF), it will return NULL. Your
definition of read_gstring_from_stdin does not handle that case (it will segfault on the
call to strlen(readp), or possibly before because the call to strip is not safe either).

I simply added a NULL check, making the definition like so:

gboolean read_gstring_from_stdin(GString *s)
{
  gchar buffer[255];
  char *readp;
  do
  {
     readp = fgets(buffer, sizeof(buffer), stdin);
     if (readp == NULL)
       return FALSE;
     strip (buffer);
     g_string_append(s, buffer);
  } while (sizeof(buffer)-1 == strlen(readp));
  return TRUE;
}

and tested the resulting package with good results. I can now feed 3000 arguments to
the xauth list testcase without problems.

I'm not sure whether the return code "FALSE" actually makes sense in the above, but
as the return code isn't checked anyway... :-)

This bug was fixed in the package libgksu - 2.0.5-1ubuntu11

---------------
libgksu (2.0.5-1ubuntu11) intrepid; urgency=low

  * debian/patches/22_increase_gksu_helper_buf.patch:
    - make the buffer in gksu-run-helper grow dynamically (LP: #173757)

 -- Michael Vogt <email address hidden> Wed, 11 Jun 2008 14:56:13 +0200

Thanks a lot for the patch review.

I updated the patch in the package (removed the gboolean return alltogether as it was not used anyway) and uploaded it to intrepid. I forgot to mention your help in the changelog :( sorry for that. I will fix that when I do a hardy version of the fix.

Just to confirm, this only affects you because you use "gksu" in non-sudo mode, right?

Thanks,
 Michael

Yes, I used update-alternatives to select "su" mode as I didn't want
to have to manually keep /etc/sudoers updated whenever new gnome
applications needing root priviliges were added.

This bug seems to be back again in version 2.0.5-1ubuntu5.2...

@Marcus: oh? compare to 2.0.5-1ubuntu5.1 ? that is odd, I looked at the diff between 5.1 and 5.2 and it looks like it does not touch the code that got added in 5.1.

Compared to what I had installed before. :-) I guess it must have been something other than
2.0.5-1ubuntu5.1 then, but update-manager kindly "upgraded" me to 2.0.5-1ubuntu5.2 which
has the bug, from something that didn't, which was what got me to react...

@Marcus: oh, I think you got a version from my PPA - that is most likely the problem. Sorry for that :/

I mean, you had a version from my PPA earlier.

Yup, you're probably right.

Thank you for reporting this bug to Ubuntu. hardy has reached EOL
(End of Life) for this package and is no longer supported. As
a result, this bug against hardy is being marked "Won't Fix".
Please see https://wiki.ubuntu.com/Releases for currently
supported Ubuntu releases.

Please feel free to report any other bugs you may find.