412889 - Crash when accessing tree columns in nsXULTreeAccessible.cpp

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Aaron Leventhal]

Crash reports:
http://crash-stats.mozilla.com/report/list?range_unit=weeks&query_search=signature&query_type=contains&version=Firefox%253A3.0b3pre%252CSeaMonkey%253A2.0a1pre%252CSunbird%253A0.6a1%252CThunderbird%253A3.0a1pre&signature=nsXULTreeAccessible%253A%253AInvalidateCache(int%252C+int)&query=Access&range_value=1

nsTreeBoxObject::GetColumns() might not set the out param yet still return NS_OK
http://mxr.mozilla.org/seamonkey/source/layout/xul/base/src/tree/src/nsTreeBoxObject.cpp#232

Comment 1

[Aaron Leventhal]

Attachment: 1) Null check cols, 2) Don't return NS_OK unless you have set the out params

I didn't check other files but in general it seems dangerous to return NS_OK without setting out params.

Comment 2

[[e-mail]]

Comment on attachment 297678 [details] [diff] [review]
1) Null check cols, 2) Don't return NS_OK unless you have set the out params

>+NS_IMETHODIMP nsTreeBoxObject::GetCellAt(PRInt32 aX, PRInt32 aY, PRInt32 *aRow, nsITreeColumn** aCol,
>+                                         nsACString& aChildElt)
> {
>+  *aRow = 0;
>+  *aCol = nsnull;
Nit: aChildElt.Truncate();

Comment 3

[Marco Zehe (:MarcoZ)]

Aaron, following our discussion on IRC, I checked nsXulTreeAccessible.cpp, and this is the only function that uses mTree before checking for its validity.
Do you still want me to file an extra bug on it, or could you put that null check in when you commit this patch?
It should follow the first if block I think, so:
>+  NS_ENSURE_TRUE(mTree && mTreeView, NS_ERROR_FAILURE); 
>   nsCOMPtr<nsITreeColumns> cols;
>   nsresult rv = mTree->GetColumns(getter_AddRefs(cols));
>-  NS_ENSURE_SUCCESS(rv, rv);
>+  NS_ENSURE_STATE(cols);

Comment 4

[Aaron Leventhal]

r+=aaron on the null check.

I'll just add the null check when I check it in.

Comment 5

[Marco Zehe (:MarcoZ)]

Verified using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3