sdaugherty at gmail.com
Wed Oct 24 15:41:59 UTC 2012
On Wed, Oct 24, 2012 at 9:48 AM, Gervase Markham <gerv at mozilla.org> wrote:
> A confidential information leak bug (bug 803992, still hidden) has
> recently been reported against a template shipped with BzAPI. There have
> been other similar bugs recently. These bugs have the common factor that a
> template called an API to get some data, and was given data that the
> currently logged-in user was not supposed to see. The template was,
> apparently, responsible for implementing whatever checks are necessary to
> filter the data, but did not do so.
> I would like to suggest that this is an anti-pattern. We should instead be
> creating APIs which are "secure by default", i.e. they return only data the
> currently-logged-in user is permitted to see. If an API for returning all
> the data is necessary, we should have a separate one, with a known marking
> so uses of those APIs in the codebase can be audited. I will suggest, as a
> straw man, the pattern that e.g. "object.things" be the secure version, and
> "object.all_things" be the insecure version.
> It will not be obvious or simple to transition fully to this kind of
> model, and we would need to do so in stages and as time permits. But it
> seems to me that sticking with the current system is going to lead to
> people - both Bugzilla devs and those making custom templates for their own
> systems - to create information leak bugs again and again. People cannot be
> expected to know that when you call object.things, you then need to
> manually remove all things which are in group 23 (or whatever) before
> displaying the list to the user.
> This sounds like very much the right approach.
I suggest that as a interim first step, rename the functions that can
return unsantitized data with some sort of prefix, so that it's easy to
audit code to find where information leaks may be happening by looking for
a particular string.
ie, hypothetically, get_all_bugs() might become
unsantitized_get_all_bugs(), and get_all_bugs() would be replaced by a stub
that logs a debug message so as not to break existing code.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the developers