On Thursday, July 28, 2011 12:51:47 am Nick Wagner wrote:
> The SLPDPredicateTreeNode structure does seem to have a problem. If we left
> the larger-sized array, guards should still be added to ensure that neither
> strncpy will step off the end of the member 'storage'. Others may have
> differing opinions, but I'd change storage to be a char* and alloc it to the
> right size (tag_len + value_len + terminating nulls), as long as we make
> sure that it's freed appropriately. We're allocating all the nodes in the
> tree anyway.
>
> --Nick
>
I think we can do that. If all are ok with it, can we make this change?
- Varun
>
> On Wed, Jul 27, 2011 at 1:27 AM, Varun Chandramohan <
> var...@linux.vnet.ibm.com> wrote:
>
> > **
> >
> > Hi Folks,
> >
> > The test team reported a bug. Iam pasting the bug analysis. They seem to
> > have found the problem as well with a temporary fix. However i they want our
> > opinion on the fix. Please advice.
> >
> > Using the following command:
> >
> > slptool findsrvs service:test$ID "(foo=value1)"
> >
> > this will generat the overflow in createPredicateParseTree doing an
> > strncpy -
> >
> > line 1656 slpd_predicate.c
> >
> > When reading the comments around "operator" it appears it is using the
> > operator
> >
> > 2 characters as a place to copy the attribute name. The attribute name can
> > be
> >
> > very large. This is the code:
> >
> > /* Finished with "operator" now - just use as temporary pointer to
> > assist
> >
> > with copying the
> >
> > * attribute name (lhs) and required value (rhs) into the node
> >
> > */
> >
> > operator = (*ppNode)->nodeBody.comparison.storage;
> >
> > strncpy(operator, lhs, lhs_len);
> >
> > operator[lhs_len] = '\0';
> >
> > operator is now the pointer of "storage" in:
> >
> > slpd_predicate.h
> >
> > typedef struct __SLPDPredicateTreeNode
> >
> > {
> >
> > SLPDPredicateTreeNodeType nodeType;
> >
> > struct __SLPDPredicateTreeNode *next; /* next node in a combination
> > */
> >
> > union {
> >
> > struct __SLPDPredicateLogicalBody
> >
> > {
> >
> > struct __SLPDPredicateTreeNode *first;
> >
> > } logical;
> >
> > struct __SLPDPredicateComparisonBody
> >
> > {
> >
> > size_t tag_len;
> >
> > char *tag_str;
> >
> > size_t value_len;
> >
> > char *value_str;
> >
> > char storage[2];
> >
> > } comparison;
> >
> > } nodeBody;
> >
> > } SLPDPredicateTreeNode;
> >
> > Copying of attributes onto 2char array fails though doesn't fail in older
> >
> > builds so I am not sure if build options or strictness has changed.
> >
> > Since there were no cleanup routines if the pointer was malloced, I just
> >
> > increased --> storage[200] and the testcase runs without fail and the area
> > will
> >
> > get freed with the structure.
> >
> > Is this the best way out?
> >
> > Regards,
> >
> > Varun
> >
> >
> >
> > ------------------------------------------------------------------------------
> > Got Input? Slashdot Needs You.
> > Take our quick survey online. Come on, we don't ask for help often.
> > Plus, you'll get a chance to win $100 to spend on ThinkGeek.
> > http://p.sf.net/sfu/slashdot-survey
> > _______________________________________________
> > Openslp-devel mailing list
> > Openslp-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/openslp-devel
> >
> >
>
------------------------------------------------------------------------------
Got Input? Slashdot Needs You.
Take our quick survey online. Come on, we don't ask for help often.
Plus, you'll get a chance to win $100 to spend on ThinkGeek.
http://p.sf.net/sfu/slashdot-survey
_______________________________________________
Openslp-devel mailing list
Openslp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openslp-devel
