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
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
