ZNC security report: CVEs for Lucid, Hardy

Oops, I removed In Progress and myself as assignee for Lucid. My bad. Meant to just do it for Hardy :P

I've attached a Lucid debdiff for the CVEs. In attempting to reverse-engineer patches for Hardy, the code-bases for ZNC differ so greatly between the Hardy and Lucid versions, it's beyond my reverse-engineering capabilities.

Reuploaded, with slightly-modified DEP3 tags which differ from the prior-uploaded patch. The code issues that prevent patching of Hardy still exist, per my last comment.

https://launchpad.net/~teward/+archive/znc-lucid contains an uploaded version of the package with the patches applied in znc. While it does not account for arm* or powerpc, it does provide i386 and amd64 build test information (only change is the changelog entry, otherwise it matches the debdiff in comment #3)

Thanks for the debdiff. Unfortunately, I cannot process it at this time because of the following:
 * cve-2010-2448.patch does not match the upstream commit (you use it++ where upstream uses ++it. Even if this is logically equivalent, it makes future maintenance more difficult). If it you require the change, can you explain here why?
 * cve-2010-2934.patch in debian/patches does not match up at all with the upstream commit in the DEP-3 comments. Is this the right patch? Perhaps the DEP-3 comment just needs to be adjusted....
 * There is trailing whitespace on this line of the changelog: "- debian/patches/cve-2010-2934.patch: modify IRCSock.cpp, " (I would just fix this myself, but you need to update the debdiff for other reasons)

cve-2010-2812.patch is considerably different than upstream, but it looks ok as a backport. Can you make the above change and resubmit the debdiff? Thanks!

Unsubscribing ubuntu-security-sponsors for now. When you resubmit your debdiff, please resubscribe.

jdstrand:

For all patches in the debdiff, the code changes had to be changed and adapted for a somewhat-differing code base. Between 0.07x (which Lucid has) and 0.09x (which is what the patches were written for, and which are all part of upstream code as part of 0.09x which is in lucid-backports), there were substantial code changes. Therefore, I tried to stick to the *original* 0.07x code as much as possible when attempting to backport the patches from the 0.09x code base. Otherwise, the next best step is to request removal of znc from lucid and hardy because of non-fixable security issues in the code (see my notes below for cve-2010-2448.patch if you won't accept leaving the code as-is).

cve-2010-2448.patch:
My reason for NOT changing it from the original code base in 0.07x is because the change to using ++it happened AFTER 0.07x was released, in a later upstream version. My other reason for not changing that from the original 0.07x code is because, the last time I checked, for security patches or other patches for Ubuntu, the goal is to only fix what's really affected, and not change the other code that is surrounding what's affected unless its necessary. Remember: this patch, and all the other patches here, were written for 0.09x. I had to adapt these patches for the 0.07x code.

I'm not going to change that code, unless you tell me otherwise, so the next debdiff will NOT contain the code change you've mentioned. But before you ask me to change the way the for loop handles incrementing itself, I'd ask you to take a look at this first, which is the ORIGINAL 0.07x line as it exists in the package in Lucid, so you can see that changing it to use ++it instead of it++, while not entirely a huge difference, would differ from the actual original code: http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/lucid/znc/lucid/view/head:/znc.cpp#L1767

======

cve-2010-2934.patch:
Oops, I think my system didn't process the "copy" command when I was copying the link from the upstream tracker, and therefore had the wrong revision number. I'll fix that.

======

cve-2010-2812.patch (although this wasn't identified as being really broken):
mdeslaur helped me out with this one. I believe sometime between 0.07x and 0.09x they changed the PING handling code to differ a bit from 0.07x. mdeslaur came to the same code-change conclusions I did, before I had finished my code base changes to attempt to match upstream, and we kept it at that, since we both came to the same code-base conclusions.

New Debdiff for Lucid:

* FIXES DEP-3 issue in cve-2010-2934.patch which points to the wrong location.
* FIXES trailing whitespace in changelog entry.

Thanks for the update. I missed that the it++ change was context and not patch, so that is ok. The changes to cve-2010-2934.patch look good too. Thanks for the patch and the explanation. ACK. I've uploaded this to the security PPA and will publish it when it finishes building.

Since znc on Ubuntu 8.04 LTS is in universe or multiverse, it is community maintained. If someone interested in fixing this bug is able, I suggest coordinating with upstream and posting a debdiff for this issue. When a debdiff is available, members of the security team will review it and publish the package. See the following link for more information: https://wiki.ubuntu.com/SecurityTeam/UpdateProcedures'

Err, znc on hardy is in universe, not 'universe or multiverse'. :)

jdstrand: No problem, glad to help where possible. Thanks for your time.

This bug was fixed in the package znc - 0.078-1ubuntu0.1

---------------
znc (0.078-1ubuntu0.1) lucid-security; urgency=low

  * SECURITY UPDATE: denial of service caused by NULL pointer dereference
    (LP: #1090195)
    - debian/patches/cve-2010-2448.patch: modify znc.cpp to prevent NULL
      pointer dereference. Based on upstream patch.
    - CVE-2010-2448
    - CVE-2010-2488
  * SECURITY UPDATE: denial of service caused by PING command without
    arguments (LP: #1090195)
    - debian/patches/cve-2010-2812.patch: modify Client.cpp to correctly
      handle PING commands that have no arguments. Based on upstream patch.
    - CVE-2010-2812
  * SECURITY UPDATE: denial of service via unknown vectors related to
    "unsafe substr() calls" (LP: #1090195)
    - debian/patches/cve-2010-2934.patch: modify IRCSock.cpp,
      modules/adminlog.cpp, modules/away.cpp, and modules/email.cpp to
      remove unsafe substr() calls. Based on upstream patch.
    - CVE-2010-2934
 -- Thomas Ward <email address hidden> Tue, 18 Dec 2012 06:29:44 +0000

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.