Closed Bug 964828 Opened 8 years ago Closed 7 years ago

Update libpurple up to 2.10.11

Categories

(Chat Core :: General, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

References

()

Details

(Whiteboard: [1.6-blocking])

Attachments

(6 files, 3 obsolete files)

+++ This bug was initially created as a clone of Bug #955042 +++

Version 2.10.8 has been released, mostly security changes, I stripped this to just the stuff we build:
libpurple:
* Fix potential crash if libpurple gets an error attempting to read a
  reply from a STUN server. (Discovered by Coverity static analysis)
  (CVE-2013-6484)
* Fix potential crash parsing a malformed HTTP response. (Discovered by
  Jacob Appelbaum of the Tor Project) (CVE-2013-6479)
* Fix buffer overflow when parsing a malformed HTTP response with
  chunked Transfer-Encoding. (Discovered by Matt Jones, Volvent)
  (CVE-2013-6485)
* Better handling of HTTP proxy responses with negative Content-Lengths.
  (Discovered by Matt Jones, Volvent)
* Fix handling of SSL certificates without subjects when using libnss.
* Fix handling of SSL certificates with timestamps in the distant future
  when using libnss. (#15586)
* Impose maximum download size for all HTTP fetches.

AIM:
* Fix untrusted certificate error.

AIM and ICQ:
* Fix a possible crash when receiving a malformed message in a Direct IM
  session.

Gadu-Gadu:
* Fix buffer overflow with remote code execution potential. Only
  triggerable by a Gadu-Gadu server or a man-in-the-middle.
  (Discovered by Yves Younan and Ryan Pentney of Sourcefire VRT)
  (CVE-2013-6487)
* Disabled buddy list import/export from/to server (it didn't work
  anymore). Buddy list synchronization will be implemented in 3.0.0.
* Disabled new account registration and password change options, as it
  didn't work either. Account registration also caused a crash. Both
  functions are available using official Gadu-Gadu website.

MSN:
* Fix NULL pointer dereference parsing headers in MSN.
  (Discovered by Fabian Yamaguchi and Christian Wressnegger of the
  University of Goettingen) (CVE-2013-6482)
* Fix NULL pointer dereference parsing OIM data in MSN.
  (Discovered by Fabian Yamaguchi and Christian Wressnegger of the
  University of Goettingen) (CVE-2013-6482)
* Fix NULL pointer dereference parsing SOAP data in MSN.
  (Discovered by Fabian Yamaguchi and Christian Wressnegger of the
  University of Goettingen) (CVE-2013-6482)
* Fix possible crash when sending very long messages. Not
  remotely-triggerable. (Discovered by Matt Jones, Volvent)

SIMPLE:
* Fix buffer overflow with remote code execution potential.
  (Discovered by Yves Younan of Sourcefire VRT) (CVE-2013-6490)

XMPP:
* Prevent spoofing of iq replies by verifying that the 'from' address
  matches the 'to' address of the iq request. (Discovered by Fabian
  Yamaguchi and Christian Wressnegger of the University of Goettingen,
  fixed by Thijs Alkemade) (CVE-2013-6483)
* Fix crash on some systems when receiving fake delay timestamps with
  extreme values. (Discovered by Jaime Breva Ribes) (CVE-2013-6477)
* Fix possible crash or other erratic behavior when selecting a very
  small file for your own buddy icon.
* Fix crash if the user tries to initiate a voice/video session with a
  resourceless JID.
* Fix login errors when the first two available auth mechanisms fail but
  a subsequent mechanism would otherwise work when using Cyrus SASL.
  (#15524)
* Fix dropping incoming stanzas on BOSH connections when we receive
  multiple HTTP responses at once. (Issa Gorissen) (#15684)

Yahoo!:
* Fix possible crashes handling incoming strings that are not UTF-8.
  (Discovered by Thijs Alkemade and Robert Vehse) (CVE-2012-6152)
* Fix a bug reading a peer to peer message where a remote user could
  trigger a crash. (CVE-2013-6481)
I'll get to this once I get purple building as an extension (bug 955009).
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Depends on: 955009
No longer depends on: 954770
Whiteboard: [1.4-wanted] → [1.6-wanted]
XMPP
 * Fix problems logging into some servers including jabber.org and chat.facebook.com. (#15879)
Summary: Update libpurple up to 2.10.8 → Update libpurple up to 2.10.9
Attached patch libpurple-2.10.9.diff (obsolete) — Splinter Review
This upgrades libpurple to 2.10.9 using the upgrade-libpurple.sh script.

The merge for a few files was done manually:
libpurple/protocols/msn/directconn.c    
libpurple/protocols/msn/msg.c    
libpurple/ssl-nss.c

In particular, the ssl changes need to be looked over and compared to what was in IB and what was changed in upstream.
Attachment #8384640 - Flags: feedback?(florian)
(In reply to Patrick Cloke [:clokep] from bug 954410, comment #64)
> (In reply to Florian Quèze [:florian] [:flo] from bug 954410, comment #62)
> > By the way, I'm pretty sure that we have worked around the lack of g_strcmp0
> > in a few places before; if we now include g_strcmp0, we should check and
> > remove our hacks the next time we update libpurple.
> I have another patch that upgrades libpurple (bug 964828), so I'll check
> that out as part of that.

I'll need to look over this. Would it be easier as a follow up patch?
(In reply to Patrick Cloke [:clokep] from comment #4)
> (In reply to Patrick Cloke [:clokep] from bug 954410, comment #64)
> > (In reply to Florian Quèze [:florian] [:flo] from bug 954410, comment #62)
> > > By the way, I'm pretty sure that we have worked around the lack of g_strcmp0
> > > in a few places before; if we now include g_strcmp0, we should check and
> > > remove our hacks the next time we update libpurple.
> > I have another patch that upgrades libpurple (bug 964828), so I'll check
> > that out as part of that.
> 
> I'll need to look over this. Would it be easier as a follow up patch?

Seems reasonable to do it separately.
Attached patch diff-current-to-2.10.7.patch (obsolete) — Splinter Review
For reference, the diff of "current" 2.10.7.
Attached patch diff-current-to-2.10.9.patch (obsolete) — Splinter Review
After applying attachment 8384640 [details] [diff] [review] this is the diff to libpurple 2.10.9.
Attached patch Use g_strcmp0Splinter Review
Simple patch that fixes the ifdefs to use g_strcmp0 again.
Attachment #8408315 - Flags: review?(florian)
Testing this on Mac and verified I could connect a couple of accounts. This fixes a couple of syntax errors from the previous patch.
Attachment #8384640 - Attachment is obsolete: true
Attachment #8384640 - Flags: feedback?(florian)
Attachment #8460670 - Flags: review?(florian)
Attachment #8408250 - Attachment is obsolete: true
Duplicate of this bug: 1088557
libpurple 2.10.10 is out

(In reply to aleth [:aleth] from bug1088557, comment #0)
> https://developer.pidgin.im/milestone/2.10.10 strangely enough doesn't link
> to as many bugs as http://pidgin.im/news/security/. Maybe they don't open a
> ticket for everything?
Summary: Update libpurple up to 2.10.9 → Update libpurple up to 2.10.10
2.10.11 is out:

    General
        Fix handling of Self-Signed SSL/TLS Certificates when using the NSS plugin (#16412)
        Improve default cipher suites used with the NSS plugin (#16262)
        Add NSS Preferences plugin which allows the SSL/TLS Versions and cipher suites to be configured (#8061) 

    Gadu-Gadu
        Fix a bug that prevented plugin to load when compiled without GnuTLS. (mancha) (#16431)
        Fix build for platforms without AF_LOCAL definition. (#16404) 

    MSN
        Fix broken login due to server change (dx, TReKiE). (#16451, #16455)
        Fail early when buddy list is unavailable instead of wasting bandwidth endlessly re-trying.
Summary: Update libpurple up to 2.10.10 → Update libpurple up to 2.10.11
Whiteboard: [1.6-wanted] → [1.6-blocking]
Attachment #8408315 - Flags: review?(florian) → review?(aleth)
Attachment #8408243 - Attachment is obsolete: true
Attachment #8408315 - Flags: review?(aleth) → review+
Comment on attachment 8460670 [details] [diff] [review]
libpurple-2.10.9.diff v2

We can also update this for the newest release first if you want...
Attachment #8460670 - Flags: review?(florian) → review?(aleth)
Comment on attachment 8460670 [details] [diff] [review]
libpurple-2.10.9.diff v2

Review of attachment 8460670 [details] [diff] [review]:
-----------------------------------------------------------------

Nothing here looks crazy to me, but I have limited experience with purple, so I may well have missed something. I'd like flo to at very least OK landing this with my review only.

::: libpurple/ssl-nss.c
@@ +38,1 @@
>  #undef HAVE_LONG_LONG /* Make Mozilla less angry. If angry, Mozilla SMASH! */

Is this a TODO you added? I'd like flo to take a look at this WIN32 change as I don't understand it.
Attachment #8460670 - Flags: review?(aleth) → review+
Flags: needinfo?(florian)
The interdiff was pretty much empty between the two versions so, we figured it made sense to push this. Florian, please take a look over this and let us know if you see any problem spots.

http://hg.mozilla.org/users/florian_queze.net/purple/rev/6629d87577c0
http://hg.mozilla.org/users/florian_queze.net/purple/rev/478aa5060a1c
http://hg.mozilla.org/users/florian_queze.net/purple/rev/34f4b9920c65
And I'll look at doing this to 2.10.11 soon.
Will that also fix bug 1098312?
(In reply to aleth [:aleth] from comment #18)
> Will that also fix bug 1098312?

Yes: https://developer.pidgin.im/query?group=status&milestone=2.10.11
Blocks: 1098312
Flags: needinfo?(florian)
This updates purplexpcom to use libpurple 2.10.11, I'll also upload the standard "diff-current-to-2.10.11.patch" with this applied afterward. I tested this by creating and connecting an AIM account.

Note that I took the liberty of syncing the msn prpl during this. We were way out of sync with it and I did not try to apply our patches to it (minus one that defines purple_mkstemp since we don't want temp files being created). Note that MSN doesn't actually connect right now though so...this is kind of not helpful.

Note that I didn't use the upgrade script to do this, I vaguely did the following:
* hg clone http://hg.pidgin.im/pidgin/main pidgin && cd pidgin
* hg up v2.10.9
* cd <purplexpcom> && DIFFCURRENTONLY=1 ./upgrade-libpurple.sh
* cd <pidgin> && hg import --no-commit <purplexpcom>/diff-current-to-2.10.9.patch
* <resolved any issues if there were any> && hg commit -m "purplexpcom v2.10.9"
* hg merge v2.10.11
* <resolve any conflicts, most were simple> && hg commit -m "Merge v2.10.11"
* cd <purplexpcom> && cp -r <pidgin>/libpurple .
* <ran a script which does a bunch of transforms to remove useless files, etc.>
* mach build extensions/purple <and fixed things until it worked>
* cp -r libpurple <pidgin>
* cd <pidgin> && hg commit --amend

I'm hoping I can then just do |hg merge v2.10.12| when it is released and essentially be done.
Attachment #8580505 - Flags: review?(florian)
Attachment #8580507 - Attachment mime type: application/x-sh → application/txt
Attachment #8580507 - Attachment mime type: application/txt → text/plain
Comment on attachment 8580505 [details] [diff] [review]
libpurple-2.10.11.diff

Review of attachment 8580505 [details] [diff] [review]:
-----------------------------------------------------------------

More of an rs/sanity check than a real review, as I'm not familiar with the gotchas to look out for.

I guess we should file a followup to remove MSN for 1.6.
Attachment #8580505 - Flags: review?(florian) → review+
http://hg.mozilla.org/users/florian_queze.net/purple/rev/2b62405a4ed1
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6
Depends on: 1189247
Blocks: 1237235
You need to log in before you can comment on or make changes to this bug.