Prevent recursions from crashing stack

[Tampa]

This is a bandaid more than a true fix, but without preventing recursions from running wild the process will eventually exceed stack and crash. This was reported as being an issue with loading restricted pages while not logged in to wifi. The actual issue is thus elsewhere in the page handling. Nonetheless recursions should be avoided if they have no bail condition that turns true before hitting stack limits. Please fix the page handling for this and perhaps rethink the approach of using recursions for parsing the pages in the first place.

[Outworldz]

POST attacks may still work with this patch as there will be content. See https://github.com/diva/diva-distribution/issues/39

[Tampa]

This pr is not to fix that issue, but to prevent recursions from rogue content being infinitely parsed. #39 describes a problem that happens as part of creating a new processing object for each of them. The problem is the entire design of this system being prone to recursions and stack overflows, it should be revised completely to avoid recursive code paths.

[Outworldz]

Both do essentially the same thing so nothing is very different. But thanks for making a patch!

[Tampa]

They are not the same thing. The attack vector isn't in estate handling or post requests on this. I'm not going to discuss the specifics here as this is a security patch and as stated the entire system needs rewriting to prevent the recursions of the various code paths from causing crashes, which will inevitably include the issue described in #39 as well.

[Outworldz]

if (recursions > 10) return string.Empty;
works the the same as
if (m_Index> 10) return string.Empty; But in fewer conditions.
But that would be arguing over minor details.

[diva]

Thanks. This will do for now.

[aiaustin]

I installed the patch for Processor.cs. Thanks. But I see an issue in the Wifi interface when, for example, using "Manage Accounts" when logged on as the grid owner. The usual single space search does not show all users... only 11 show. I assume that's the recursion limit (+1) which causes the problem.

I simply set the recursion limit set in the patch higher temporarily.

Just while looking at this, is there a real reason to limit searches there to a min of 3 characters? Would 2 be more reasonable and work fine for smaller grids anyway?

[Outworldz]

500 On windows does not run out of stack space. But it limites the Avatars list to 500 users.

[Tampa]

Again, this is a bandaid, temporary, preventing the worst cases. The actual fix for this is code that doesn't rely on recursion to parse data and has clear conditions on what it parses and what is discarded to prevent things from running away. That's something @diva needs to look into as I don't have a full overview of the code or data it works with.

I suspect or at least hope that redesign of this function or the larger content serving system is in the works and will be merged soon.

@aiaustin The reason search is limited is because most names contain more than 3 letters and the quantity of results grows exponentially from 3 to 2 characters.

[Outworldz]

Edited: Depending on the patch, its limited N lines (10) which is very little, and certainly not enough. A JSON response to a Ajax call and a javascript table with search and paging would be what I would do here.