Full_Name: Nalin Dahyabhai Version: 1.2.10 OS: Linux 2.2.14 URL: http://people.redhat.com/nalin/patches/openldap-1.2.9-strings.patch Submission from: (NULL) (207.175.42.207) While fixing a misconfigured set of defaults for a security errata, we took a look at some of the string-handling code. A number of locations in the OpenLDAP sources use strcat() on fixed-size buffers. We changed the more obvious places where this was happening to use strncat() instead. Please feel free to review the changes and include them in subsequent releases if you find them useful.
> Full_Name: Nalin Dahyabhai > Version: 1.2.10 > OS: Linux 2.2.14 > URL: http://people.redhat.com/nalin/patches/openldap-1.2.9-strings.patch > Submission from: (NULL) (207.175.42.207) > > > While fixing a misconfigured set of defaults for a security errata, we took a > look > at some of the string-handling code. A number of locations in the OpenLDAP > sources > use strcat() on fixed-size buffers. We changed the more obvious places where > this > was happening to use strncat() instead. Please feel free to review the changes > and > include them in subsequent releases if you find them useful. > > > I looked at some of this patch file. It is noteable at the top that you have a RedHat copyright -- I don't know if it would make it incompatable for contribution back to the OpenLDAP codebase, but it seems for minor changes like those that such a copyright is unfounded. Also, a number of those changes are unnecessary, and just add extra code. For example, when you start with a 255 char buffer, and you are squeezing extra spaces out, but definition you are not going to overrun that buffer. Randy
On Thu, Apr 27, 2000 at 04:38:06PM -0500, Randy Kunkee wrote: > I looked at some of this patch file. It is noteable at the top that > you have a RedHat copyright -- I don't know if it would make it incompatable > for contribution back to the OpenLDAP codebase, but it seems for minor changes > like those that such a copyright is unfounded. I agree completely. I was told by Ben via email that some sort of permission statement would need to be added, so I attached the usual text we use when it needs to be there. The only examples I could find on the web site were all prefaced with a note that the developer team aren't lawyers. If there's a specific license (or none) you'd prefer, I'd be happy to change it. (The version of the patch in the latest source package in Raw Hide has no such statement, actually; we usually don't bother with them for patches.) > Also, a number of those changes are unnecessary, and just add extra code. > For example, when you start with a 255 char buffer, and you are squeezing > extra spaces out, but definition you are not going to overrun that buffer. This patch was almost an afterthought, and parts of it are almost certain to be redundant, as I wasn't too choosy in which parts of the code to make changes in. If there is logic to prevent overflows in some other way, I didn't trace through it. If data passed in that gets passed to strcat() or strcpy() is length checked before being passed in, I missed that, too. By all means please use the useful parts (if there are any), and chuck the rest. You won't be hurting my feelings. Thanks, Nalin
There are no significant copyright issues with this submission and I do not have any specific objection to the patch [or portions thereof] being applied by the committer [Ben, in this case]. At 09:29 PM 4/27/00 GMT, kunkee@neosoft.com wrote: >Also, a number of those changes are unnecessary, and just add extra code. >For example, when you start with a 255 char buffer, and you are squeezing >extra spaces out, but definition you are not going to overrun that buffer. As I haven't reviewed the code in great detail, I will leave it to Ben's best judgement as to what to commit. I will, however, state that I believe it generally wise to write "safe" code, even overly "safe" code (where it doesn't have a significant impact on performance). Kurt
At 09:45 PM 4/27/00 GMT, nalin@redhat.com wrote: >On Thu, Apr 27, 2000 at 04:38:06PM -0500, Randy Kunkee wrote: >> I looked at some of this patch file. It is noteable at the top that >> you have a RedHat copyright -- I don't know if it would make it incompatable >> for contribution back to the OpenLDAP codebase, but it seems for minor changes >> like those that such a copyright is unfounded. > >I agree completely. Per my previous note, an explicit copyright notice is generally better than no copyright notice at all. Depending on the submission, we may require an explicit notice. We generally do not require explicit notices on simple patches. I will interact with submitters and committers as needed. >I was told by Ben via email that some sort of permission >statement would need to be added, so I attached the usual text we use when >it needs to be there. The only examples I could find on the web site were >all prefaced with a note that the developer team aren't lawyers. If there's >a specific license (or none) you'd prefer, I'd be happy to change it. (The >version of the patch in the latest source package in Raw Hide has no such >statement, actually; we usually don't bother with them for patches.) I have no issue with the copyright you attached, I believe it to be compatible with our release and devel licenses. I generally prefer simple notices, such as those on the our website. Kurt
changed state Open to Feedback moved from Incoming to Software Enhancements
moved from Software Enhancements to Software Bugs
changed notes
changed state Feedback to Closed
Pending Ben's review