Add security fixes from upstream

Bug #1319089 reported by Kent Baxley
258
This bug affects 1 person
Affects Status Importance Assigned to Milestone
openwsman (Ubuntu)
Fix Released
Undecided
Unassigned
Trusty
Fix Released
Undecided
Kent Baxley
Utopic
Fix Released
Undecided
Unassigned

Bug Description

The upstream maintainer for openwsman has added in a bunch of security fixes after our security team conducted an audit of the code. There are still a few patches left to go, but, I would like to go ahead and include what's already upstream into the 14.04 release:

ws_xml_make_default_prefix() can overflow buf parameter via sprintf()
wsmc_create_request() potential buf[20] overflow via WSMAN_ACTION_RENEW
LocalSubscriptionOpUpdate() unchecked fopen()
Incorrect order of sanity guards in wsman_get_fault_status_from_doc()
Unchecked memory allocation in wsman_init_plugins(), p->ifc
Unchecked memory allocation in mem_double(), newptr
Unchecked memory allocation in dictionary_new(), d, d->val, d->key, d->hash
Unchecked memory allocation in u_error_new(), *error
sighup_handler() in wsmand.c uses unsafe functions in a signal handler
Support SHA512 password encoding, use safe_cmp to prevent brute-force
attacks
increase password upper limit to 128 characters (from 64)

Tags: patch

Related branches

Revision history for this message
Kent Baxley (kentb) wrote :
information type: Private Security → Public Security
Revision history for this message
Seth Arnold (seth-arnold) wrote :

Kent, thanks for taking on this task; your debdiff currently has all the .pc/ files from having quilt patches applied, which complicates reviewing the debdiff. Can you regenerate it with the quilt patches popped?

To issue a security update for this, we should have CVEs for any of these patches that may be security issues; do you know if Klaus has CVE numbers already assigned or not? If not, I can ask for CVEs on oss-security. (I'm hoping Klaus has, because perhaps some of these are simple bug fixes.)

Thanks

Revision history for this message
Kent Baxley (kentb) wrote :

Hey Seth,

Thanks for the review. I'll drop the quilt patches to make the diff easier to look at.

Klaus does not have any CVE numbers generated, so, I believe they'll need to be requested.

Revision history for this message
Kent Baxley (kentb) wrote :

Seth, here's the debdiff with the quilt patches popped. Let me know if this is what you need or if I can get you anything else on this. Thanks!

Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "bug-1310989.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Thanks for updating the patch! A couple of comments:

* I don't see that CVEs are assigned for any of these issues or any requests to oss-security for them? Do you know if upstream has CVEs assigned? If not, can someone request them on http://oss-security.openwall.org/wiki/mailing-lists/oss-security? (we can publish the update without CVE assignments)
* Normally we would break out all the fixes into discrete patches (one for each CVE/security issue) so that it is easier to review, backout and understand
* each patch should follow DEP-5 which makes review much easier and less error prone. Combined with splitting out the patches, each patch would include the upstream commit to make it easier to find (you did include a link to a page that lists the commits and while it is workable enough, it isn't as clear as it should be)
* the patch to src/lib/wsman-subscription-repository.c is quite different than 09c3fcf4d209f6890eb9cb9e554bff637eae73b5
* the patch to src/lib/u/iniparser.c needs both 89dabd4582e3fbb88328dd780e89baf6efb4ad3f and 638abcbf5faa97ccb2c3ab15faeb2f2cc9363b56, but the DEP-5 comments and changelog entry is not clear on this point (though you include it)
 * the changelog entry does not contain 'SECURITY UPDATE'

The real blocker on this is the lack of information on the changes to src/lib/wsman-subscription-repository.c (I would've likely sponsored it otherwise), however since you are going to update the packaging anyway, can you:
* break out the patches, one per security issue/CVE and include full Origin and Description for each (ie, follow DEP-5, being sure to include the Origin for supporting patches). If this is burdensome, you can instead add the Origin to debian/changelog for each patch and any supporting patches
* ask upstream to request CVEs (you could do this yourself if desired)
* update debian/changelog to follow https://wiki.ubuntu.com/SecurityTeam/UpdatePreparation#Packaging (specifically, include 'SECURITY UPDATE', any assigned CVE identifiers and all Origin information if it isn't included in the DEP-5 comments of the patches

Changed in openwsman (Ubuntu):
assignee: nobody → Kent Baxley (kentb)
status: New → In Progress
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Unsubscribing ubuntu-security-sponsors for now. Please resubscribe when resubmitting. Thanks again!

Revision history for this message
Kent Baxley (kentb) wrote :

Ok. I'll start working on this and have something early next week. Thanks for all the pointers.

Revision history for this message
Kent Baxley (kentb) wrote :

Sent requests in this morning. I'll start breaking things out once I receive CVE numbers:

http://www.openwall.com/lists/oss-security/2014/05/19/8

Kent Baxley (kentb)
description: updated
Revision history for this message
Kent Baxley (kentb) wrote :

I've updated my branch to break out each patch individually as well as *only* include relevant stuff for security fixes.

I was unable to make any headway with upstream with regard to getting help filing CVE's. Since these patches are already in upstream, I'm hoping that we can include them anyway now that more details about each fix are included. The changelog lists each patch and each patch file itself contains details as well as a link to the commit itself.

Hopefully this will be enough. Definitely let me know if anything else needs addressing.

Thanks.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Looks good except for some very minor trailing whitespace in debian/changelog. I can adjust this for the upload. Thanks for the update! I've uploaded this to the security PPA and will push to the archive once it is done building.

Changed in openwsman (Ubuntu):
status: In Progress → Fix Committed
Changed in openwsman (Ubuntu Trusty):
status: New → Fix Committed
assignee: nobody → Kent Baxley (kentb)
Changed in openwsman (Ubuntu Utopic):
assignee: Kent Baxley (kentb) → nobody
status: Fix Committed → Triaged
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Once it is published, I will pocket copy to utopic.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package openwsman - 2.4.3-0ubuntu4.1

---------------
openwsman (2.4.3-0ubuntu4.1) trusty-security; urgency=low

  * SECURITY UPDATE: Add security fixes from upstream openwsman (LP: #1319089)
    - debian/patches/ws-xml-make-default-prefix-buff-overflow-fix.patch:
      ws_xml_make_default_prefix() can overflow buf parameter via sprintf()
    - debian/patches/wsmc-create-request-fix-buff-overflow.patch:
      wsmc_create_request() potential buf[20] overflow via WSMAN_ACTION_RENEW
    - debian/patches/LocalSubscriptionOpUpdate-fix-fopen.patch:
      address LocalSubscriptionOpUpdate() unchecked fopen()
    - debian/patches/wsman-get-fault-status-sanity-guard-fix.patch:
      Fix incorrect order of sanity guards in wsman_get_fault_status_from_doc()
    - debian/patches/mem-allocation-wsman-init-plugins-fix.patch:
      Fix unchecked memory allocation in wsman_init_plugins(), p->ifc
    - debian/patches/mem-allocation-mem-double-newptr-fix.patch:
      Fix unchecked memory allocation in mem_double(), newptr
    - debian/patches/mem-allocation-dictionary-new-fix.patch:
      Fix unchecked memory allocation in dictionary_new(), d, d->val, d->key,
      d->hash
    - debian/patches/mem-allocation-u-error-new-fix.patch:
      Fix unchecked memory allocation in u_error_new(), *error
    - debian/patches/remove-unsafe-debug-call-from-sighup-handler.patch:
      sighup_handler() in wsmand.c use of unsafe functions in a signal handler
    - debian/patches/SHA512-password-fixes.patch:
      Support SHA512 password encoding, use safe_cmp to prevent brute-force
      attacks
    - debian/patches/increase-password-upper-limit.patch:
      increase password upper limit to 128 characters (from 64)
 -- Kent Baxley <email address hidden> Fri, 06 Jun 2014 12:55:02 -0500

Changed in openwsman (Ubuntu Trusty):
status: Fix Committed → Fix Released
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Copied to utopic

Changed in openwsman (Ubuntu Utopic):
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.