<div class="gmail_quote">On Wed, Oct 24, 2012 at 9:48 AM, Gervase Markham <span dir="ltr"><<a href="mailto:gerv@mozilla.org" target="_blank">gerv@mozilla.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.<br>

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

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

<br>
Comments?<br>
<br></blockquote><div>This sounds like very much the right approach. </div><div><br></div><div>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. </div>
<div><br></div><div>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.</div><div><br></div>
<div>Thoughts?</div><div><br></div><div><br></div><div><br></div></div>