Hide Forgot
+++ This bug was initially created as a clone of Bug #747852 +++ => openswan-2.6.32/programs/pluto/ike_alg.c:87 when ike_alg_get_encrypter returns NULL (line 82) there is NULL dereference on 87 when asking for enc_desc->keyminlen. There is probably else keyword missing on line 87 (else-if statement). a) if ike_alg_get_encrypter fails on line 82 b) enc_desc is dereferenced on line 87. This problem was newly found in RHEL 5.8 by Coverity difference scan on RHEL packages 5.7 → 5.8. version: openswan-2.6.32 --- Additional comment from avagarwa on 2011-10-21 09:54:23 EDT --- I looked at the code, and that seems like a false alarm. As the code checks for enc_desc is null or not before dereferencing it and return false. So this seems like a not a bug. --- Additional comment from sgrubb on 2011-10-21 10:18:51 EDT --- Avesh, there is a bug there enc_desc = ike_alg_get_encrypter(ealg); suppose this is NULL if (!enc_desc) { It goes in here /* failure: encrypt algo must be present */ snprintf(ugh_buf, ugh_buf_len, "encrypt algo not found"); ret = FALSE; it does not return, so it hits the next line } if ((key_len) && ((key_len < enc_desc->keyminlen) segfaults here because it probably should be else if so that it skips over this block --- Additional comment from avagarwa on 2011-10-21 10:24:29 EDT --- Steve you are right, I mislooked it thinking ret=false is a return statement. i think that needs to be corrected.
Since RHEL 6.2 External Beta has begun, and this bug remains unresolved, it has been rejected as it is not proposed as exception or blocker. Red Hat invites you to ask your support representative to propose this request, if appropriate and relevant, in the next release of Red Hat Enterprise Linux.
Created attachment 580658 [details] Added defects in openswan-2.6.32-14 Hello, there were revealed new defects in openswan between releases openswan-2.6.32-13 and openswan-2.6.32-14 1. Possible NULL dereference of variable narrowed_tsi => openswan-2.6.32/programs/pluto/ikev2_child.c:1188|1192|1197 - on line 1179 is done a check for null value of 'narrowed_tsi' but in the else branche is this variable dereferenced. 2. Unchecked return value (which may (in error case) become negative) => openswan-2.6.32/programs/pluto/ikev2_parent.c:2068|2069 This possibly negative values are passed as a parameters to function 'ikev2_verify_ts' which expects unsigned value. 3. Off by one error in linux/net/ipsec/addrtot.c => openswan-2.6.32/linux/net/ipsec/addrtot.c Should there be done something like that: | --- a/linux/net/ipsec/addrtot.c | +++ b/linux/net/ipsec/addrtot.c | @@ -79,7 +79,7 @@ size_t dstlen; | if (n == 0) { | bad: | dst[0]='\0'; | - strncat(dst, "<invalid>", dstlen); | + strncat(dst, "<invalid>", dstlen - 1); | return sizeof("<invalid>"); | } There is a lot of warnings due to use of DBG_log macro with addrtot inside on openswan-2.6.32/programs/pluto/ikev2_child.c:1099|1106|... where is used addrtot() with given length of buffer 'sizeof(buffer)' -- no space left for '\0'. This is probably not so dangerous but it would be better to fix it sometime in future. 4. Weird working with 'struct traffic_selector' list. => openswan-2.6.32/programs/pluto/ikev2_child.c:136 There is done check for NULL value. Could there even occur non-NULL? If yes, there may be quite problem on line 140 or 141 -- it blindly removes reference to next item (which may be non null). Anyway, coverity claims that there may occur resource leak and it is imo true if the 'ts_this' parameter comes to this function non-null. (the very similar problem on line 156). I'm attaching coverity log with defects added by the transition to the new release. Thanks, Pavel Development: These errors are mentioned just as a warning and it depends on you whether they will be fixed. These defects can be not as dangerous and/or not as prioritized to be fixed in 6.3 - so feel free to move these defects to 6.4 or forget about this comment if we don't need to fix these at all. Quality engineering: These issues were found by static analysis tool and we can't provide any reproducer for these. We are able to rescan package once these problems are fixed. Please check these tests as SanityOnly (just check that patches for the issues and check if nothing unexpected was added by the commit). If you want to check the new package with Coverity yourself, feel free to use covscan tool (https://engineering.redhat.com/trac/CoverityScan/wiki/covscan).
Created attachment 580659 [details] List of all defects in openswan-2.6.32-14.
Since RHEL 6.3 External Beta has begun, and this bug remains unresolved, it has been rejected as it is not proposed as exception or blocker. Red Hat invites you to ask your support representative to propose this request, if appropriate and relevant, in the next release of Red Hat Enterprise Linux.
These will be addressed in the RHEL-7 libreswan package. We have run a fresh coverity scan there and are working through all of the remaining issues. (some of these issues mentioned here were already fixed between openswn 2.6.32 and 2.6.38, but we won't be shipping openswan 2.6.38 in RHEL-7) (libreswan is a fork from openswan 2.6.38) If you believe these fixes are urgent for you, please ask your support representative to propose this request for a RHEL-6 update.
Re-opened this one and put the patch in for a 6.5 release for "missing else" issue and coverity number 3 issue. the other issues related to ikev2 traffic selectors involve a lot of code change, as those were changed extensively in libreswan. I'm tempted to leave that alone for now. for libreswan in rhel7, we're also going through all coverity reported issues.
This bug will be review by patch review only and hence there is no need for a test coverage.
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. http://rhn.redhat.com/errata/RHBA-2013-1718.html