Bug 696561 - Coverity scan revealed issues
Summary: Coverity scan revealed issues
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: cryptsetup-luks
Version: 5.7
Hardware: Unspecified
OS: Unspecified
low
low
Target Milestone: rc
: ---
Assignee: Milan Broz
QA Contact: Release Test Team
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-04-14 10:24 UTC by Michal Luscon
Modified: 2013-03-01 04:10 UTC (History)
9 users (show)

Fixed In Version: cryptsetup-luks-1.0.3-8.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-07-21 07:04:25 UTC
Target Upstream Version:


Attachments (Terms of Use)
Proposed solution - dereferencing null mk variable (54 bytes, patch)
2011-04-14 10:24 UTC, Michal Luscon
no flags Details | Diff
Proposed solution - sector_size(fd) negative return value (51 bytes, patch)
2011-04-14 10:27 UTC, Michal Luscon
no flags Details | Diff
Proposed solution - resource leak (185 bytes, patch)
2011-04-14 10:29 UTC, Michal Luscon
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2011:0987 0 normal SHIPPED_LIVE cryptsetup-luks bug fix update 2011-07-20 15:45:04 UTC

Description Michal Luscon 2011-04-14 10:24:34 UTC
Created attachment 492020 [details]
Proposed solution - dereferencing null mk variable

Description of problems:

1. Function "sector_size(fd)" may return negative value to variable "int bsize" which is passed to a parameter of "read" that cannot be negative in file /lib/utils.c:261.


2. In the case of return null by malloc null variable mk is dereferenced on line #49 in /luks/keymanage.c.


3. Resource leak: Function "__crypt_luks_format" in /lib/setup.c:499 returns -1 without deallocation variable mk.

Additional info:

These defects were not present in current supported version of cryptsetup-luks package.

Comment 1 Michal Luscon 2011-04-14 10:27:21 UTC
Created attachment 492021 [details]
Proposed solution - sector_size(fd) negative return value

Comment 2 Michal Luscon 2011-04-14 10:29:14 UTC
Created attachment 492024 [details]
Proposed solution - resource leak

Comment 3 Milan Broz 2011-04-14 11:54:32 UTC
cryptsetup version in RHEL5 is full of bugs which  are fixed upstream (except one). But because there was huge code changes, it cannot be easily backported.

Included patches are kind of tradeoff here, it was minimal change to fix real bugs but if RHEL management wants to take risk of replacing very uncommon fails with some possible regressions after 5.7 devel deadline, why not.

BTW one of the proposed patch does not fix the real problem (negative bsize handling in write_lseek_blockwise is wrong even upstream, you missed char frontPadBuf[bsize] on stack).

Comment 6 Ondrej Vasik 2011-04-14 12:21:01 UTC
Milan, I agree cryptsetup version in RHEL5 has plenty of bugs/defects fixed by upstream - but these defects were not present in the 5.6 version of cryptsetup - so are new in RHEL-5. I fully agree we should be very careful with fixing defects already present in RHEL-5 - it is old code and even the small change could change the behaviour.

Comment 8 Milan Broz 2011-04-14 12:47:06 UTC
(In reply to comment #6)
> Milan, I agree cryptsetup version in RHEL5 has plenty of bugs/defects fixed by
> upstream - but these defects were not present in the 5.6 version of cryptsetup
> - so are new in RHEL-5.

The new patch fixes _fatal_ buffer overflow problem for long keys (using backported patch from some of the intermediate cryptsetup version, see bug #678011). So it fixed serious bug and introduced some other (because I know what code does I can do much more better analysis than coverity which has no idea of high level code view).

I am just surprised that you are reporting it now and not before devel freeze.

Anyway, if PM is OK with fixing that, I'll do that, but there is risk of regressions.

Comment 15 Milan Broz 2011-04-19 15:11:14 UTC
Fixed in cryptsetup-luks-1.0.3-8.el5.

Comment 18 errata-xmlrpc 2011-07-21 07:04:25 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0987.html


Note You need to log in before you can comment on or make changes to this bug.