Taylor Blau <me@xxxxxxxxxxxx> writes: > Since this is a local-only exploit, a user would have to be tricked into > running such a query by an adversary. Even if they were successfully > tricked into running the malicious query, the blast radius is limited to > a local stack overflow, which does not have meaningful paths to remote > code execution, arbitrary memory reads, or any more grave security > concerns. > ... So the difference in practice is if we make a controlled call to die() or just let it crash? It still does sound worthwhile thing to do to make sure we make a controlled death. But ... > +static int get_nth_ancestor_max_depth = 4096; > +static int get_nth_ancestor_curr_depth; ... do we have a lock at a much higher level that prevents multiple name-to-oid look-ups from running simultaneously, or something similar, to make use of this static counter safe? I am not offhand sure how safe it is to assume that we'd always be single-threaded. This variable leaves a bad taste in my mouth. I am not offhand sure how hard it is to count the depth per callpath; get_oid_1() is the sole caller of get_nth_ancestor(), so if you rename the former into a separate helper with a new "recursion_depth" parameter, create a thin wrapper around it that starts the recursion at depth 0 and have everybody else (i.e., peel_onion() and get_oid_with_context_1()) call it, and have get_nth_ancestor increment (and die as needed) the counter, would that be sufficient to ensure that we count the depth per call invocation? Thanks.
