gajim code execution and sql injection

[Julian Taylor]

Imported from Debian bug http://bugs.debian.org/668038:

Package: gajim
Severity: grave
Tags: security

Hi,

Two security issues were reported in gajim: one user assisted code
execution and one an SQL injection:

- https://trac.gajim.org/ticket/7031
- https://trac.gajim.org/ticket/7034

They are fixed in gajim 0.15-1, which is in unstable and I've asked the
release team to increase the urgency value so it reaches testing sooner.
Can you please verify if the version in squeeze is indeed affected by
these issues and if so, are you able to provide an updated package? If
not, please also let the security team know.

Cheers,
Thijs

[Tyler Hicks (tyhicks)]

Hi Julian - Thanks for the debdiffs! I've reviewed them and have compiled some feedback...

Debdiff review:

* New package versions are wrong. For example, the Oneiric version should be
  '0.14.1-1ubuntu2'. Please see the version examples at:
  https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Update_the_packaging

* Being picky, if I reference the patch origin's URL in the patch tags, I
  typically don't reference the URL in the changelog, too. This matches the
  changelog template at the link above.

* As an FYI, when we receive a merge request for security sponsoring, we
  generate a debdiff using the latest source package (possibly from the
  -security or -updates pockets) and proceed to use the debdiff from there. So,
  we generally prefer to get debdiffs from the start, but that isn't
  documented. I wanted to mention it incase it is easier on you to provide a
  debdiff.

Patch backport review:

* The backported CVE-2012-2085.patch is in all three releases is missing
  gajim.thread_interface(p.wait) call in else block of exec_command()

* The natty and lucid debdiffs seem to have a missing "jid_tuple = (jid_id,)"
  in the else block of CVE-2012-2086.patch in chunk @ 654.

Additionally, please comment on the level of testing you've done with these patches applied. Thanks!

[Julian Taylor (jtaylor)]

thanks for the thorough review.

> * New package versions are wrong. For example, the Oneiric version should be
> '0.14.1-1ubuntu2'. Please see the version examples at:
> https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Update_the_packaging

they provide an upgrade path, they are just a bit longer than minimal. Does this matter?
also according to the wiki it shouldn't it be 0.14.1-1ubuntu1.1

> * The backported CVE-2012-2085.patch is in all three releases is missing
> gajim.thread_interface(p.wait) call in else block of exec_command()

> * The natty and lucid debdiffs seem to have a missing "jid_tuple = (jid_id,)"
> in the else block of CVE-2012-2086.patch in chunk @ 654.

fixed the issues and forwarded them to debian where they also exist.

[Julian Taylor (jtaylor)]

testing just basic startup and connection tests. I checked that the tmpfile regression that occured in debian does not happend and also caught and fixed another issue in the debian patch and forwarded that earlier.

[Tyler Hicks (tyhicks)]

> they provide an upgrade path, they are just a bit longer than minimal. Does this matter?

Yes - it results in uglier version numbers. We only want to use the extended version numbers when necessary.

> also according to the wiki it shouldn't it be 0.14.1-1ubuntu1.1

You're right. Sorry for the bad advice. :)

I'll update the changelog version numbers myself, perform some local checks, and kick off the builds. Thanks for the updated debdiffs!

[Launchpad Janitor (janitor)]

This bug was fixed in the package gajim - 0.14.1-1ubuntu1.1

---------------
gajim (0.14.1-1ubuntu1.1) oneiric-security; urgency=low

  * SECURITY UPDATE: assisted code execution (LP: #992618)
    - debian/patches/CVE-2012-2085.patch: fix subprocess call to prevent
      shell escape from via crafted messages
      https://trac.gajim.org/changeset/bc296e96ac10
    - CVE-2012-2085
  * SECURITY UPDATE: sql injection in logging code (LP: #992618)
    - debian/patches/CVE-2012-2086.patch: use a prepated statement
      https://trac.gajim.org/changeset/bfd5f94489d8
    - CVE-2012-2086
  * SECURITY UPDATE: insecure tmpfile creation (LP: #992613)
    - debian/patches/CVE-2012-2093.patch: use safe tmpfile functions
      when convering LaTeX IM messages to png images
      Thanks to Nico Golde
    - CVE-2012-2093
 -- Julian Taylor <email address hidden> Thu, 10 May 2012 17:48:34 -0700

[Launchpad Janitor (janitor)]

This bug was fixed in the package gajim - 0.13.4-3ubuntu2.1

---------------
gajim (0.13.4-3ubuntu2.1) natty-security; urgency=low

  * SECURITY UPDATE: assisted code execution (LP: #992618)
    - debian/patches/CVE-2012-2085.patch: fix subprocess call to prevent
      shell escape from via crafted messages
      https://trac.gajim.org/changeset/bc296e96ac10
    - CVE-2012-2085
  * SECURITY UPDATE: sql injection in logging code (LP: #992618)
    - debian/patches/CVE-2012-2086.patch: use a prepated statement
      https://trac.gajim.org/changeset/bfd5f94489d8
    - CVE-2012-2086
  * SECURITY UPDATE: insecure tmpfile creation (LP: #992613)
    - debian/patches/CVE-2012-2093.patch: use safe tmpfile functions
      when convering LaTeX IM messages to png images
      Thanks to Nico Golde
    - CVE-2012-2093
 -- Julian Taylor <email address hidden> Thu, 10 May 2012 17:48:45 -0700

[Launchpad Janitor (janitor)]

This bug was fixed in the package gajim - 0.13-0ubuntu2.1

---------------
gajim (0.13-0ubuntu2.1) lucid-security; urgency=low

  * SECURITY UPDATE: assisted code execution (LP: #992618)
    - debian/patches/CVE-2012-2085.dpatch: fix subprocess call to prevent
      shell escape from via crafted messages
      https://trac.gajim.org/changeset/bc296e96ac10
    - CVE-2012-2085
  * SECURITY UPDATE: sql injection in logging code (LP: #992618)
    - debian/patches/CVE-2012-2086.dpatch: use a prepated statement
      https://trac.gajim.org/changeset/bfd5f94489d8
    - CVE-2012-2086
  * SECURITY UPDATE: insecure tmpfile creation (LP: #992613)
    - debian/patches/CVE-2012-2093.dpatch: use safe tmpfile functions
      when convering LaTeX IM messages to png images
      Thanks to Nico Golde
    - CVE-2012-2093
 -- Julian Taylor <email address hidden> Thu, 10 May 2012 17:48:53 -0700

[David Avsajanishvili (avsd05)]

Hi all!

Just installed the security update - and now getting exception each time Gajim receives or sends a message:

{{{

Traceback (most recent call last):
  File "/usr/share/gajim/src/common/xmpp/idlequeue.py", line 528, in _process_events
    return IdleQueue._process_events(self, fd, flags)
  File "/usr/share/gajim/src/common/xmpp/idlequeue.py", line 393, in _process_events
    obj.pollin()
  File "/usr/share/gajim/src/common/xmpp/transports_nb.py", line 414, in pollin
    self._do_receive()
  File "/usr/share/gajim/src/common/xmpp/transports_nb.py", line 600, in _do_receive
    self._on_receive(received)
  File "/usr/share/gajim/src/common/xmpp/transports_nb.py", line 614, in _on_receive
    self.on_receive(data)
  File "/usr/share/gajim/src/common/xmpp/dispatcher_nb.py", line 452, in dispatch
    handler['func'](session, stanza)
  File "/usr/share/gajim/src/common/connection_handlers.py", line 1615, in _messageCB
    jid, invite, tim)
  File "/usr/share/gajim/src/common/connection_handlers.py", line 1628, in _on_message_decrypted
    session.received(frm, msgtxt, tim, encrypted, msg)
  File "/usr/share/gajim/src/session.py", line 268, in received
    first, nickname, msg, focused], advanced_notif_num)
  File "/usr/share/gajim/src/notify.py", line 308, in notify
    helpers.play_sound(snd_event)
  File "/usr/share/gajim/src/common/helpers.py", line 738, in play_sound
    play_sound_file(path_to_soundfile)
  File "/usr/share/gajim/src/common/helpers.py", line 802, in play_sound_file
    exec_command(command)
  File "/usr/share/gajim/src/common/helpers.py", line 387, in exec_command
    gajim.thread_interface(p.wait)
TypeError: __init__() takes exactly 5 arguments (2 given)

}}}

It seems, the patch applied to v. 0.14 is partial. Please, take a look: https://trac.gajim.org/changeset/bc296e96ac10

[Tyler Hicks (tyhicks)]

On 2012-05-15 02:59:14, David Avsajanishvili wrote:
> Just installed the security update - and now getting exception each time
> Gajim receives or sends a message:

What Ubuntu release are you running? You mention gajim version 0.14 below,
which is only used in Oneiric. Can you confirm that you're running
Oneiric?

> It seems, the patch applied to v. 0.14 is partial. Please, take a look:
> https://trac.gajim.org/changeset/bc296e96ac10

I'm not seeing why you feel that the patch Ubuntu is carrying is
partial. IMO, it looks like a valid backport of the upstream patch. Can
you be more specific about the differences that you're noticing?

Julian - Can you please take a look at this?

[undefined (undefined)]

on lucid i receive the same traceback as avsd05 (or close enough; i'm too lazy to perfectly diff the two).

the missing patch: https://trac.gajim.org/changeset/12863.

the reason: exec_command() calls thread_interface() with only one argument (p.wait, which is the command to execute in the new thread; the second argument referenced in the traceback is python's implicit "self" argument to class methods). the version of ThreadInterface in lucid has all its __init__ arguments as required, where the revision the security patch was pulled from had only the first argument (after the implicit self) as required. revision 12863 includes the updated interface with the optional arguments.

applying the patch to src/gajim.py (the file referenced in changeset 12863 doesn't exist in this earlier revision due to later refactoring), while ignoring whitespace, messes up indentation, so i've created the attached patch.

[Tyler Hicks (tyhicks)]

Thanks for determining the problem, undefined! I've got new packages building locally. I'll make sure that I got all of the packaging changes right and then upload the new versions to the Ubuntu Security Proposed PPA in hopes that undefined and avsd05 can give them a quick test. I'll comment when the packages are ready for testing.

[Tyler Hicks (tyhicks)]

Actually, it looks like jtaylor might have beaten me to the punch. From here on, please refer to bug #999629 for tracking the resolution of the regression.