Bug#725757: opu: zabbix/1:1.8.2-1squeeze5
Control: tags -1 + confirmed
On Fri, 2013-10-11 at 13:16 +1100, Dmitry Smirnov wrote:
> On Fri, 11 Oct 2013 05:00:41 you wrote:
> > For the record, that all comes to "8 files changed, 6906 insertions(+),
> > 5 deletions(-)", which is considerably more than I was expecting, given
> > how close we are to the update window closing.
> >
> > A lot of it appears to be a (possibly over-cautious) belt and braces
> > approach to
> >
> > > * CVE-2013-5743: fixed SQL injection vulnerability.
> >
> > escaping basically every use of a string anywhere near an SQL statement.
> > I do hope that someone's actually checked that none of those additions
> > of zbx_dbstr() introduces any bugs; I certainly don't know what any of
> > the variables might contain in order to judge. :-(
>
> Well, that's a heavy patch but it was specifically made by upstream
> developers for the very version of Zabbix that we have in Squeeze. I
> applied it as-is without any modifications. If you wish we can ask
> upstream for comments.
>
> In Squeeze I tested instance of Zabbix-1.8.2 with this patch applied
> and couldn't see any regressions. I doubt there is anything more I
> could possibly do to ensure the safety of this patch.
That's encouraging at least; thanks.
> > There's also
> >
> > > * CVE-2011-3263: prevent zabbix_agentd DoS attack with vfs.file.cksum.
> >
> > patches/ZBX-3794+ZBX-3830.patch | 540 +++
> >
> > There's quite a lot of noise in that patch, of the general form
[...]
Apologies if I'm missing something, but I'm also not sure what the
benefit of this (from zbx_strdup2()) is, beyond a simple strdup:
++ for (retry = 10; 0 < retry && NULL == ptr; ptr = strdup(str), retry--)
++ ;
particularly given that there's no delay between the tries. I guess it
doesn't hurt though.
> Apologies if this patch is not perfect. This particular patch I
> backported long time ago and (unlike SQL injections that I find
> difficult to test) I verified that patch fixes DoS attack on
> "vfs.file.cksum" on Zabbix-1.8.2/Squeeze. I'm quite confident that it
> works as expected. I don't remember whether renaming of those
> variables were necessary to apply other patches...
Thanks for the confirmation.
> Adam, please advise if you feel more confident with uploading just
> patch for SQL injection and leaving all other changes behind.
Other than the noise, I'm generally happy with the other patches; the
SQL fix is the one that worries me most, so just uploading that one
would be odd. :-)
Based on another review and your comments above, I'm going to cross my
fingers, say "please go ahead" and hope everything turns out fine.
Please bear in mind that the upload window for 6.0.8 closes this
weekend.
Regards,
Adam
Reply to: