Lionel Elie Mamane
2006-Mar-13 12:28 UTC
[Secure-testing-team] Re: Bug#342943: only kronolith2 fixed
package kronolith reopen 349261 tags 349261 +help thanks On Sat, Jan 21, 2006 at 03:56:30PM -0500, Joey Hess wrote:> clone 342943 -1 > reassign -1 kronolith > thanks> This security hole was fixed in kronolith2, but the kronolith > package is still present in unstable and still, presumably, has this > hole.Thank you for warning us. However, kronolith 1 is not maintained upstream anymore and no patch for this issue is available from upstream. I''ve tried to backport the upstream patch for kronolith 2, but most files touched don''t actually exist in kronolith 1, as well as a sizeable part of the code touched in the files that do exist. Here is my measle backport attempt, but I''d really like someone that understands the issue to review it and see if nothing has been left out. Do we have someone of that calibre (and willing to do it) available in Debian? Maybe it is getting time to dump Horde2 from etch/sid, as the pain to keeping it in has actually increased significantly. What do you think about this Ola & Jose? The problem stays for sarge, though. -- Lionel -------------- next part -------------- diff --recursive -uN kronolith-1.1.4/templates/data/export.inc kronolith-1.1.4.deb/templates/data/export.inc --- kronolith-1.1.4/templates/data/export.inc 2003-02-13 00:23:36.000000000 +0100 +++ kronolith-1.1.4.deb/templates/data/export.inc 2006-01-22 08:09:09.000000000 +0100 @@ -113,7 +113,7 @@ <td> <?php $end_year_match = $start_year_match; -$end_month_match = $start_month_match +1; +$end_month_match = $start_month_match + 1; $end_day_match = $start_day_match; $end_hour_match = $start_hour_match; $end_min_match = $start_min_match; diff --recursive -uN kronolith-1.1.4/templates/delete/delete.inc kronolith-1.1.4.deb/templates/delete/delete.inc --- kronolith-1.1.4/templates/delete/delete.inc 2003-04-17 18:37:13.000000000 +0200 +++ kronolith-1.1.4.deb/templates/delete/delete.inc 2006-01-22 08:22:49.000000000 +0100 @@ -3,21 +3,24 @@ $url = Kronolith::addParameter(''month.php'', ''month='' . $month); $url = Kronolith::addParameter($url, ''year='' . $year); $url = Horde::applicationUrl($url, true); + } else { + // Escape URLs that came from client-side input. + $url = htmlspecialchars($url); } ?> <form action="<?php echo Horde::applicationUrl(''deleventaction.php'') ?>" method="post" target="_self" name="delete"> -<input type="hidden" name="year" value="<?php if (isset($year)) echo $year ?>" /> -<input type="hidden" name="month" value="<?php if (isset($month)) echo $month ?>" /> -<input type="hidden" name="mday" value="<?php if (isset($day)) echo $day ?>" /> +<input type="hidden" name="year" value="<?php if (isset($year)) echo htmlspecialchars($year) ?>" /> +<input type="hidden" name="month" value="<?php if (isset($month)) echo htmlspecialchars($month) ?>" /> +<input type="hidden" name="mday" value="<?php if (isset($day)) echo htmlspecialchars($day) ?>" /> <input type="hidden" name="url" value="<?php echo $url ?>" /> -<input type="hidden" name="eventID" value="<?php echo $event->getID() ?>" /> +<input type="hidden" name="eventID" value="<?php echo htmlspecialchars($event->getID()) ?>" /> <center> <table border="0" cellspacing="0" cellpadding="4" align="center"> <!-- header --> <tr class="header"> - <td align="left" class="header"><b><?php echo sprintf(_("Delete %s"), $event->getTitle()) ?></b></td> + <td align="left" class="header"><b><?php echo sprintf(_("Delete %s"), htmlspecialchars($event->getTitle())) ?></b></td> </tr> <!-- description --> @@ -31,7 +34,7 @@ <input type="submit" class="button" name="current" value="<?php echo _("Current") ?>" /> <input type="submit" class="button" name="future" value="<?php echo _("Future") ?>" /> <input type="submit" class="button" name="all" value="<?php echo _("All") ?>" /> - <input type="submit" class="button" name="cancel" value="<?php echo _("Cancel") ?>" onclick="self.location = ''<?php echo $url; ?>''; return false;" /> + <input type="submit" class="button" name="cancel" value="<?php echo _("Cancel") ?>" onclick="self.location = ''<?php echo htmlspecialchars($url); ?>''; return false;" /> </td> </tr> diff --recursive -uN kronolith-1.1.4/templates/delete/one.inc kronolith-1.1.4.deb/templates/delete/one.inc --- kronolith-1.1.4/templates/delete/one.inc 2003-04-17 18:37:13.000000000 +0200 +++ kronolith-1.1.4.deb/templates/delete/one.inc 2006-01-22 08:16:23.000000000 +0100 @@ -6,18 +6,18 @@ } ?> <form action="<?php echo Horde::applicationUrl(''deleventaction.php'') ?>" method="post" name="delete"> -<input type="hidden" name="year" value="<?php if (isset($year)) echo $year ?>" /> -<input type="hidden" name="month" value="<?php if (isset($month)) echo $month ?>" /> -<input type="hidden" name="mday" value="<?php if (isset($day)) echo $day ?>" /> -<input type="hidden" name="url" value="<?php echo $url ?>" /> -<input type="hidden" name="eventID" value="<?php echo $event->getID() ?>" /> +<input type="hidden" name="year" value="<?php if (isset($year)) echo htmlspecialchars($year) ?>" /> +<input type="hidden" name="month" value="<?php if (isset($month)) echo htmlspecialchars($month) ?>" /> +<input type="hidden" name="mday" value="<?php if (isset($day)) echo htmlspecialchars($day) ?>" /> +<input type="hidden" name="url" value="<?php echo htmlspecialchars($url) ?>" /> +<input type="hidden" name="eventID" value="<?php echo htmlspecialchars($event->getID()) ?>" /> <center> <table border="0" cellspacing="0" cellpadding="4" align="center"> <!-- header --> <tr class="header"> - <td align="left" class="header"><b><?php echo sprintf(_("Delete %s"), $event->getTitle()) ?></b></td> + <td align="left" class="header"><b><?php printf(_("Delete %s"), htmlspecialchars($event->getTitle())) ?></b></td> </tr> <!-- description --> @@ -29,7 +29,7 @@ <tr> <td align="left"> <input type="submit" class="button" name="delete" value="<?php echo _("Delete") ?>" /> - <input type="submit" class="button" name="cancel" value="<?php echo _("Cancel") ?>" onclick="self.location = ''<?php echo $url; ?>''; return false;" /> + <input type="submit" class="button" name="cancel" value="<?php echo _("Cancel") ?>" onclick="self.location = ''<?php echo htmlspecialchars($url); ?>''; return false;" /> </td> </tr> diff --recursive -uN kronolith-1.1.4/templates/edit/edit.inc kronolith-1.1.4.deb/templates/edit/edit.inc --- kronolith-1.1.4/templates/edit/edit.inc 2004-07-22 17:52:59.000000000 +0200 +++ kronolith-1.1.4.deb/templates/edit/edit.inc 2006-01-22 08:34:15.000000000 +0100 @@ -27,16 +27,16 @@ ?> <form action="<?php echo ($event->isInitialized() ? ''editeventaction.php'' : ''addeventaction.php'') ?>" method="post" name="event"> <?php Horde::pformInput() ?> -<input type="hidden" name="year" value="<?php if (isset($year)) echo $year ?>" /> -<input type="hidden" name="month" value="<?php if (isset($month)) echo $month ?>" /> -<input type="hidden" name="mday" value="<?php if (isset($day)) echo $day ?>" /> -<input type="hidden" name="timestamp" value="<?php if (isset($timestamp)) echo $timestamp ?>" /> +<input type="hidden" name="year" value="<?php if (isset($year)) echo htmlspecialchars($year) ?>" /> +<input type="hidden" name="month" value="<?php if (isset($month)) echo htmlspecialchars($month) ?>" /> +<input type="hidden" name="mday" value="<?php if (isset($day)) echo htmlspecialchars($day) ?>" /> +<input type="hidden" name="timestamp" value="<?php if (isset($timestamp)) echo htmlspecialchars($timestamp) ?>" /> <input type="hidden" name="new_category" value="" /> <?php if (isset($url)): ?> -<input type="hidden" name="url" value="<?php echo $url ?>" /> +<input type="hidden" name="url" value="<?php echo htmlspecialchars($url) ?>" /> <?php endif; ?> <?php if ($event->isInitialized()): ?> -<input type="hidden" name="eventID" value="<?php echo $event->getID() ?>" /> +<input type="hidden" name="eventID" value="<?php echo htmlspecialchars($event->getID()) ?>" /> <?php endif; ?> <center> @@ -55,7 +55,7 @@ <input type="submit" class="button" name="saveAsNew" value="<?php echo _("Save As New") ?>" onclick="return checkCategory();" /> <input type="submit" class="button" name="delete" value="<?php echo _("Delete Event") ?>" onclick="self.location = ''<?php echo $delurl; ?>''; return false;" /> <?php endif; ?> - <input type="submit" name="cancel" class="button" value="<?php echo _("Cancel") ?>" onclick="self.location = ''<?php echo $cancelurl; ?>''; return false;" /> + <input type="submit" name="cancel" class="button" value="<?php echo _("Cancel") ?>" onclick="self.location = ''<?php echo $cancelurl ?>''; return false;" /> <input type="button" name="resetButton" class="button" value="<?php echo _("Reset to Defaults") ?>" onclick="document.event.reset(); updateWday(''start_wday''); updateWday(''end_wday'');" /> </td> </tr> @@ -335,7 +335,7 @@ </td> <?php if (($count % 3 == 2) || ($count == count($keywords) - 1)): ?> </tr> - <?php endif; $count++; ?> + <?php endif; ++$count; ?> <?php endforeach; ?> </table> </td> @@ -350,7 +350,7 @@ <input type="submit" class="button" name="saveAsNew" value="<?php echo _("Save As New") ?>" onclick="return checkCategory();" /> <input type="submit" class="button" name="delete" value="<?php echo _("Delete Event") ?>" onclick="self.location = ''<?php echo $delurl; ?>''; return false;" /> <?php endif; ?> - <input type="submit" name="cancel" class="button" value="<?php echo _("Cancel") ?>" onclick="self.location = ''<?php echo $cancelurl; ?>''; return false;" /> + <input type="submit" name="cancel" class="button" value="<?php echo _("Cancel") ?>" onclick="self.location = ''<?php echo $cancelurl ?>''; return false;" /> <input type="button" name="resetButton" class="button" value="<?php echo _("Reset to Defaults") ?>" onclick="document.event.reset(); updateWday(''start_wday''); updateWday(''end_wday'');" /> </td> </tr> diff --recursive -uN kronolith-1.1.4/templates/view/view.inc kronolith-1.1.4.deb/templates/view/view.inc --- kronolith-1.1.4/templates/view/view.inc 2003-04-17 18:37:13.000000000 +0200 +++ kronolith-1.1.4.deb/templates/view/view.inc 2006-01-22 08:24:08.000000000 +0100 @@ -137,10 +137,10 @@ </tr> <?php foreach ($keyword_list as $cat => $list): ?> <tr> - <td align="right" class="light" valign="top"><b><?php echo $cat ?> </b></td> + <td align="right" class="light" valign="top"><b><?php echo htmlspecialchars($cat) ?> </b></td> <td align="left" class="text" colspan="3"> <?php foreach ($list as $entry): ?> - <?php echo $entry ?><br /> + <?php echo htmlspecialchars($entry) ?><br /> <?php endforeach; ?> </td> </tr>
Anthony DeRobertis
2006-Mar-13 12:28 UTC
[Secure-testing-team] Re: Bug#342943: only kronolith2 fixed
Neil McGovern wrote:> A fairly odd bug. It only affects the app if REGISTER_GLOBALS is on, > however, the app requires REGISTER_GLOBALS :|Isn''t this in and of itself a problem due to CVE-2005-3390. Is that finally going to be fixed in Sarge? http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=336645 certainly hinted otherwise.
Neil McGovern
2006-Mar-13 12:28 UTC
[Secure-testing-team] Re: Bug#342943: only kronolith2 fixed
On Sat, Jan 28, 2006 at 09:23:31PM +0100, Martin Schulze wrote:> Neil McGovern wrote: > > On Sun, Jan 22, 2006 at 11:35:15AM +0100, Martin Schulze wrote: > > > Lionel Elie Mamane wrote: > > > > I''ve tried to backport the upstream patch for kronolith 2, but most > > > > files touched don''t actually exist in kronolith 1, as well as a > > > > sizeable part of the code touched in the files that do exist. Here is > > > > my measle backport attempt, but I''d really like someone that > > > > understands the issue to review it and see if nothing has been left > > > > out. Do we have someone of that calibre (and willing to do it) > > > > available in Debian? > > > > > > I''ve taken a look at the patch, and several lines contain changes not > > > suitable for a security update, i.e. fix different potential bugs or > > > change the code. I''m attaching the patch. More eyes checking would > > > be appreciated. > > > > > > > A fairly odd bug. It only affects the app if REGISTER_GLOBALS is on, > > however, the app requires REGISTER_GLOBALS :| > > > > I''ll do an audit of the code and try and find anything left over when I > > get home later. > > Any news on this? >Sorry for the delay. I haven''t managed to find any more bugs relating to this particular security hole that isn''t fixed by the previous patch in this bug report. kronolith seems to be fairly badly coded wrt security issues though. I''d suggest depreciating kronolith1 and forcing people on to kronolith2, whcih although only a little better, is actually supported upstream. Cheers, Neil -- __ .` `. neilm@debian.org | Application Manager : :'' ! ---------------- | Secure-Testing Team member ''. `- gpg: B345BDD3 | Webapps Team member `- Please don''t cc, I''m subscribed to the list
Ola Lundqvist
2006-Mar-13 12:28 UTC
[Secure-testing-team] Re: Bug#349261: Bug#342943: only kronolith2 fixed
Hello On Sun, Jan 29, 2006 at 09:33:12PM +0100, Lionel Elie Mamane wrote:> On Sun, Jan 29, 2006 at 06:15:23PM +0000, Neil McGovern wrote: > > On Sat, Jan 28, 2006 at 09:23:31PM +0100, Martin Schulze wrote: > >> Neil McGovern wrote: > > >>> A fairly odd bug. It only affects the app if REGISTER_GLOBALS is > >>> on, however, the app requires REGISTER_GLOBALS :| > > >>> I''ll do an audit of the code and try and find anything left over > >>> when I get home later. > > >> Any news on this? > > > Sorry for the delay. > > > I haven''t managed to find any more bugs relating to this particular > > security hole that isn''t fixed by the previous patch in this bug > > report. kronolith seems to be fairly badly coded wrt security > > issues though. I''d suggest depreciating kronolith1 and forcing > > people on to kronolith2, whcih although only a little better, is > > actually supported upstream. > > The problem is that kronolith2 depends on version 3 of the horde > framework (rather than version 2), that the two versions of horde > cannot meaningfully cooperate and there are still some horde2 > applications that have not been ported to horde3. Basically, upstream > has abandoned horde2 before they ported all their OWN code to horde3. > > So dropping horde2 is a regression, which explains why we haven''t done > it yet. But I''m toying with the idea, as we cannot meaningfully > support it anyway. Ola, your opinion?If kronolith1 (named kronolith) can not be fixed, and is not supported at all by upstream I think we should drop it. Regards, // Ola> -- > Lionel > >-- --------------------- Ola Lundqvist --------------------------- / opal@debian.org Annebergsslingan 37 \ | opal@lysator.liu.se 654 65 KARLSTAD | | +46 (0)54-10 14 30 +46 (0)70-332 1551 | | http://www.opal.dhs.org UIN/icq: 4912500 | \ gpg/f.p.: 7090 A92B 18FE 7994 0C36 4FE4 18A1 B1CF 0FE5 3DD9 / ---------------------------------------------------------------
Lionel Elie Mamane
2006-Mar-13 12:28 UTC
[Secure-testing-team] Bug#342943: only kronolith2 fixed
On Sun, Jan 29, 2006 at 06:15:23PM +0000, Neil McGovern wrote:> On Sat, Jan 28, 2006 at 09:23:31PM +0100, Martin Schulze wrote: >> Neil McGovern wrote:>>> A fairly odd bug. It only affects the app if REGISTER_GLOBALS is >>> on, however, the app requires REGISTER_GLOBALS :|>>> I''ll do an audit of the code and try and find anything left over >>> when I get home later.>> Any news on this?> Sorry for the delay.> I haven''t managed to find any more bugs relating to this particular > security hole that isn''t fixed by the previous patch in this bug > report. kronolith seems to be fairly badly coded wrt security > issues though. I''d suggest depreciating kronolith1 and forcing > people on to kronolith2, whcih although only a little better, is > actually supported upstream.The problem is that kronolith2 depends on version 3 of the horde framework (rather than version 2), that the two versions of horde cannot meaningfully cooperate and there are still some horde2 applications that have not been ported to horde3. Basically, upstream has abandoned horde2 before they ported all their OWN code to horde3. So dropping horde2 is a regression, which explains why we haven''t done it yet. But I''m toying with the idea, as we cannot meaningfully support it anyway. Ola, your opinion? -- Lionel
Martin Schulze
2006-Mar-13 12:28 UTC
[Secure-testing-team] Re: Bug#342943: only kronolith2 fixed
Skipped content of type multipart/mixed-------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 189 bytes Desc: Digital signature Url : http://lists.alioth.debian.org/pipermail/secure-testing-team/attachments/20060122/edfd2118/attachment.pgp
Florian Weimer
2006-Mar-13 12:28 UTC
[Secure-testing-team] Re: Bug#342943: only kronolith2 fixed
* Martin Schulze:> I''ve taken a look at the patch, and several lines contain changes not > suitable for a security update, i.e. fix different potential bugs or > change the code. I''m attaching the patch. More eyes checking would > be appreciated.This one seems only safe when magic_quotes_gpc is enabled: - <input type="submit" [...] onclick="self.location = ''<?php echo $url; ?>''; return false;" /> + <input type="submit" [...] onclick="self.location = ''<?php echo htmlspecialchars($url); ?>''; return false;" /> (htmlspecialchars does not quote single quotes, and even if it did, it would not really help because the HTML should be reversed before the JavaScript parser runs.) It''s probably not a real problem because everybody runs with magic_quotes_gpc enabled, though. Apart from the issues in your diff, there seem to be others. Is anybody familiar with the HORDE framework (at that version) and can explain how variables are handled internal? There seems to be some kind of register_globals reimplementation.
Neil McGovern
2006-Mar-13 12:28 UTC
[Secure-testing-team] Re: Bug#342943: only kronolith2 fixed
On Sun, Jan 22, 2006 at 11:35:15AM +0100, Martin Schulze wrote:> Lionel Elie Mamane wrote: > > I''ve tried to backport the upstream patch for kronolith 2, but most > > files touched don''t actually exist in kronolith 1, as well as a > > sizeable part of the code touched in the files that do exist. Here is > > my measle backport attempt, but I''d really like someone that > > understands the issue to review it and see if nothing has been left > > out. Do we have someone of that calibre (and willing to do it) > > available in Debian? > > I''ve taken a look at the patch, and several lines contain changes not > suitable for a security update, i.e. fix different potential bugs or > change the code. I''m attaching the patch. More eyes checking would > be appreciated. >A fairly odd bug. It only affects the app if REGISTER_GLOBALS is on, however, the app requires REGISTER_GLOBALS :| I''ll do an audit of the code and try and find anything left over when I get home later. Neil -- __ .` `. neilm@debian.org | Application Manager : :'' ! ---------------- | Secure-Testing Team member ''. `- gpg: B345BDD3 | Webapps Team member `- Please don''t cc, I''m subscribed to the list -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 189 bytes Desc: Digital signature Url : http://lists.alioth.debian.org/pipermail/secure-testing-team/attachments/20060122/72655cef/attachment.pgp
Martin Schulze
2006-Mar-13 12:28 UTC
[Secure-testing-team] Re: Bug#342943: only kronolith2 fixed
Neil McGovern wrote:> On Sun, Jan 22, 2006 at 11:35:15AM +0100, Martin Schulze wrote: > > Lionel Elie Mamane wrote: > > > I''ve tried to backport the upstream patch for kronolith 2, but most > > > files touched don''t actually exist in kronolith 1, as well as a > > > sizeable part of the code touched in the files that do exist. Here is > > > my measle backport attempt, but I''d really like someone that > > > understands the issue to review it and see if nothing has been left > > > out. Do we have someone of that calibre (and willing to do it) > > > available in Debian? > > > > I''ve taken a look at the patch, and several lines contain changes not > > suitable for a security update, i.e. fix different potential bugs or > > change the code. I''m attaching the patch. More eyes checking would > > be appreciated. > > > > A fairly odd bug. It only affects the app if REGISTER_GLOBALS is on, > however, the app requires REGISTER_GLOBALS :| > > I''ll do an audit of the code and try and find anything left over when I > get home later.Any news on this? Regards, Joey -- Computers are not intelligent. They only think they are. Please always Cc to me when replying to me on the lists.
Lionel Elie Mamane
2006-Mar-13 12:28 UTC
[Secure-testing-team] Re: Bug#349261: Bug#342943: only kronolith2 fixed
On Thu, Feb 09, 2006 at 10:47:28AM +0100, Martin Schulze wrote:> Ola Lundqvist wrote:>>>> I''d suggest depreciating kronolith1 and forcing people on to >>>> kronolith2, whcih although only a little better, is actually >>>> supported upstream.>>> The problem is that kronolith2 depends on version 3 of the horde >>> framework (rather than version 2), that the two versions of horde >>> cannot meaningfully cooperate and there are still some horde2 >>> applications that have not been ported to horde3. Basically, >>> upstream has abandoned horde2 before they ported all their OWN >>> code to horde3.>>> So dropping horde2 is a regression, which explains why we haven''t >>> done it yet. But I''m toying with the idea, as we cannot >>> meaningfully support it anyway. Ola, your opinion?>> If kronolith1 (named kronolith) can not be fixed, and is not >> supported at all by upstream I think we should drop it.> It seems to be removed already.Yes, that story spurred us into requesting removal from unstable of the whole horde2 suite. This still leaves the security update to stable, though. -- Lionel
Martin Schulze
2006-Mar-13 12:28 UTC
[Secure-testing-team] Re: Bug#349261: Bug#342943: only kronolith2 fixed
Ola Lundqvist wrote:> > > I haven''t managed to find any more bugs relating to this particular > > > security hole that isn''t fixed by the previous patch in this bug > > > report. kronolith seems to be fairly badly coded wrt security > > > issues though. I''d suggest depreciating kronolith1 and forcing > > > people on to kronolith2, whcih although only a little better, is > > > actually supported upstream. > > > > The problem is that kronolith2 depends on version 3 of the horde > > framework (rather than version 2), that the two versions of horde > > cannot meaningfully cooperate and there are still some horde2 > > applications that have not been ported to horde3. Basically, upstream > > has abandoned horde2 before they ported all their OWN code to horde3. > > > > So dropping horde2 is a regression, which explains why we haven''t done > > it yet. But I''m toying with the idea, as we cannot meaningfully > > support it anyway. Ola, your opinion? > > If kronolith1 (named kronolith) can not be fixed, and is not supported > at all by upstream I think we should drop it.It seems to be removed already. Regards, Joey -- Everybody talks about it, but nobody does anything about it! -- Mark Twain Please always Cc to me when replying to me on the lists.