Hi all, I have sent a PR <https://github.com/apache/fineract/pull/357> [1] resolving the rest of the issues of FINERACT-436 <https://issues.apache.org/jira/browse/FINERACT-436> [2]. For tracking purpose I have created a new ticket at FINERACT-470 <https://issues.apache.org/jira/browse/FINERACT-470>. This is the second PR connected with the previously merged PR <https://github.com/apache/fineract/pull/343> [3].
Please review and let me know if any thing is needed to be changed. I am moving on to fixing the FINERACT-437 <https://issues.apache.org/jira/browse/FINERACT-437>. [1] https://github.com/apache/fineract/pull/357 [2] https://issues.apache.org/jira/browse/FINERACT-436 [3] https://github.com/apache/fineract/pull/343 Thanks & Regard On Thu, May 11, 2017 at 1:58 AM, Ed Cable <[email protected]> wrote: > Thisura, I have cc'd Mark on this email > > On May 10, 2017 14:07, "Thisura Philips" <[email protected]> wrote: > > > Hi Nikhil, > > > > I am really glad to get selected to GOSC 2017. I have being looking on > TOIF > > a little bit in the past few days. First I will complete the SONAR > issues. > > > > Since Mark Reynolds is the mentor of the project, I would like to get him > > in this thread too. Couldn't find his email though. Please some one > provide > > me the email of Mark. > > > > Thanks & Regards > > > > > > > > > > > > On Sun, Apr 30, 2017 at 7:41 AM, Thisura Philips <[email protected]> > > wrote: > > > > > Hi all, > > > > > > I have sent the PR [1] > > > <https://github.com/apache/incubator-fineract/pull/343> including the > > > changes needed to resolve the FINERACT-436 [2] > > > <https://issues.apache.org/jira/browse/FINERACT-436> in accounting and > > > infrastructure packages. Please see the updated scanning report [3] > > > <https://docs.google.com/spreadsheets/d/1uLk3YPcjnXk7RqF8etsTzIuN59CDU > > 6sgBxpZul__1V4/> > > > . > > > > > > It would be great if you can review and give feedback to continue > fixing > > > other vulnerabilities. > > > > > > I have done the LAPSE scanning for a part of accounting package as > well. > > > Will send the results soon. > > > > > > [1] https://github.com/apache/incubator-fineract/pull/343 > > > [2] https://issues.apache.org/jira/browse/FINERACT-436 > > > [3] https://docs.google.com/spreadsheets/d/ > > 1uLk3YPcjnXk7RqF8etsTzIuN59CDU > > > 6sgBxpZul__1V4/ > > > > > > Thanks & Regards > > > > > > On Sat, Apr 29, 2017 at 8:12 AM, Thisura Philips <[email protected] > > > > > wrote: > > > > > >> Hi Myrle, > > >> > > >> Agreed with the concern in doing an API change. > > >> > > >> The problem is any one can change the values in these arrays since > they > > >> are accessible public. It is nice if we can protect these arrays, > (set) > > as > > >> they are mutable. > > >> But as we know we can't use protected or private access modifiers in > an > > >> interface. It would be perfect if we can move the Sets in to the > > respective > > >> classes where they are used. > > >> > > >> supportedParameters = > org.apache.fineract.accounting. > > >> provisioning.serialization.ProvisioningEntriesDefinitionJ > > sonDeserializer > > >> (Used only in this class) > > >> > > >> PROVISIONING_ENTRY_PARAMETERS => org.apache.fineract.accounting > > >> .provisioning.api.ProvisioningEntriesApiResource (Used only in this > > >> class) > > >> > > >> ALL_PROVISIONING_ENTRIES => org.apache.fineract.accounting > > >> .provisioning.api.ProvisioningEntriesApiResource (Used only in this > > >> class) > > >> > > >> Since all of these sets are used in the mentioned classes only, we can > > >> make them private. > > >> > > >> > > >> Thanks & Regards > > >> > > >> > > >> On Fri, Apr 28, 2017 at 5:49 PM, Myrle Krantz <[email protected]> > wrote: > > >> > > >>> By constant class Thisura, do you mean a final class with a private > > >>> constructor? > > >>> > > >>> It would be possible to change > > >>> org.apache.fineract.accounting.provisioning.constant.Provisi > > >>> oningEntriesApiConstants. > > >>> This would be a breaking change though. Given that the interface has > > >>> no methods, and no one is likely to have derived from it, there's > > >>> probably no code to break by changing it, but you still have to > answer > > >>> the question "Why would I change that?" > > >>> > > >>> Yes, I know that Bloch ("Effective Java", 2nd Ed., Chapter 19) said > > >>> it's bad. But his arguments only make sense when referring to an > > >>> interface which contains methods. This interface doesn't. So given > > >>> that Apache Fineract is communicated with over a REST interface, what > > >>> harm does this interface cause that would justify making an > > >>> API-breaking change (though a minor one) to remediate the situation? > > >>> > > >>> Best Regards, > > >>> Myrle > > >>> > > >>> > > >>> On Sun, Apr 23, 2017 at 8:00 PM, Thisura Philips < > [email protected] > > > > > >>> wrote: > > >>> > Hi all, > > >>> > > > >>> > I have done some of the fixes for FINERACT-436 > > >>> > <https://issues.apache.org/jira/browse/FINERACT-436>. Please see > the > > >>> > updated document > > >>> > <https://drive.google.com/open?id=1uLk3YPcjnXk7RqF8etsTzIuN5 > > >>> 9CDU6sgBxpZul__1V4> > > >>> > . > > >>> > > > >>> > Is there any particular reason to > > >>> > have org.apache.fineract.accounting.provisioning.constant.Provisi > > >>> oningEntriesApiConstants > > >>> > as an interface. It is true that variables in interfaces are > static, > > >>> final > > >>> > by default. But yet this can cause the following vulnerabilities. > > >>> > > > >>> > - MITRE, CWE-582 <http://cwe.mitre.org/data/ > definitions/582.html> > > - > > >>> > Array Declared Public, Final, and Static > > >>> > - MITRE, CWE-607 <http://cwe.mitre.org/data/ > definitions/607.html> > > - > > >>> > Public Static Final Field References Mutable Object > > >>> > > > >>> > > > >>> > Can't we change this to a constant class? ASFAIK this is the > correct > > >>> way to > > >>> > maintain a set of constants. > > >>> > > > >>> > Thanks & Regards > > >>> > > > >>> > > > >>> > On Sun, Apr 23, 2017 at 5:53 PM, Thisura Philips < > > [email protected] > > >>> > > > >>> > wrote: > > >>> > > > >>> >> Hi all, > > >>> >> > > >>> >> I have created two tickets [1][2] to track the fixes for security > > >>> >> vulnerabilities reported by sonar. > > >>> >> Thanks & Regards. > > >>> >> [1] https://issues.apache.org/jira/browse/FINERACT-436 > > >>> >> [2] https://issues.apache.org/jira/browse/FINERACT-437 > > >>> >> > > >>> >> On Fri, Apr 21, 2017 at 10:31 AM, Thisura Philips < > > >>> [email protected]> > > >>> >> wrote: > > >>> >> > > >>> >>> Hi all, > > >>> >>> > > >>> >>> As per the long discussion in the thread "[Mifos-developer] > > >>> Application > > >>> >>> for GSOC 2017( Static Analysis of Apache Fineract )", I have > > >>> >>> > > >>> >>> * done the static analysis with SonarQube > > >>> >>> * generated the vulnerability report, - sonarlint report [1] > > >>> >>> <https://drive.google.com/open?id=0B6WV3fK5Tak7Uy1uOWk0SW81Wm8>, > > >>> >>> sonarqube <https://drive.google.com/open > > >>> ?id=0B6WV3fK5Tak7OHJENF9oZFE2X2c> report > > >>> >>> [2] <https://drive.google.com/open?id= > 0B6WV3fK5Tak7OHJENF9oZFE2X2c > > > > > >>> >>> * summarized > > >>> >>> <https://drive.google.com/open?id=1TdwwHM2K1gMb6qILEX7gmzU8d > > >>> VXcHGBdh569aFJfB2U> > > >>> >>> [3] the types of vulnerabilities, > > >>> >>> * attended each of those vulnerabilities to check whether they > are > > >>> not > > >>> >>> false positives and > > >>> >>> * prepared the checklist [4] > > >>> >>> <https://drive.google.com/open?id=1uLk3YPcjnXk7RqF8etsTzIuN5 > > >>> 9CDU6sgBxpZul__1V4> > > >>> >>> of vulnerabilities with fixes > > >>> >>> > > >>> >>> All the reports which are generated using different plugins, > tools > > >>> can be > > >>> >>> found here [5] > > >>> >>> <https://drive.google.com/open?id=0B6WV3fK5Tak7ZVJkVGV3WVZ3OE0>. > > >>> >>> > > >>> >>> Now we can go ahead and do the necessary changes to fix the > > reported > > >>> >>> vulnerabilities in the codebase. I am looking forward to creating > > >>> tickets > > >>> >>> for each type of issues reported in summary. > > >>> >>> > > >>> >>> We need to verify the problems (vulnerabilities[4] > > >>> >>> <https://drive.google.com/open?id=1uLk3YPcjnXk7RqF8etsTzIuN5 > > >>> 9CDU6sgBxpZul__1V4>) > > >>> >>> and solutions that I have suggested in the summary [3] > > >>> >>> <https://drive.google.com/open?id=1TdwwHM2K1gMb6qILEX7gmzU8d > > >>> VXcHGBdh569aFJfB2U> > > >>> >>> . > > >>> >>> > > >>> >>> According to the findings [4] > > >>> >>> <https://drive.google.com/open?id=1uLk3YPcjnXk7RqF8etsTzIuN5 > > >>> 9CDU6sgBxpZul__1V4>, > > >>> >>> I will create two tickets for co-related fixes for each topic > > (type > > >>> of > > >>> >>> vulnerability) such as > > >>> >>> > > >>> >>> * Mutable fields should not be "public static" && "enum" fields > > >>> should > > >>> >>> not be publicly mutable && "public static" fields should be > > constant > > >>> >>> * Generic exceptions should never be thrown && Throwable and > Error > > >>> should > > >>> >>> not be caught > > >>> >>> > > >>> >>> Expect the community ideas regarding this to validate the > suggested > > >>> >>> solutions. > > >>> >>> > > >>> >>> [1] https://drive.google.com/open?id= > 0B6WV3fK5Tak7Uy1uOWk0SW81Wm8 > > >>> >>> [2] https://drive.google.com/open?id= > 0B6WV3fK5Tak7OHJENF9oZFE2X2c > > >>> >>> [3] https://drive.google.com/open?id=1TdwwHM2K1gMb6qILEX7gmz > > >>> >>> U8dVXcHGBdh569aFJfB2U > > >>> >>> [4] https://drive.google.com/open?id=1uLk3YPcjnXk7RqF8etsTzI > > >>> >>> uN59CDU6sgBxpZul__1V4 > > >>> >>> [5] https://drive.google.com/open?id= > 0B6WV3fK5Tak7ZVJkVGV3WVZ3OE0 > > >>> >>> > > >>> >>> Thanks & regards > > >>> >>> -- > > >>> >>> T.T.C Philips (BSc.Eng (Undergrad)) > > >>> >>> Computer Science and Engineering, > > >>> >>> Sri Lanka Institute of Information Technology(SLIIT) > > >>> >>> > > >>> >>> > > >>> >>> > > >>> >>> > > >>> >> > > >>> >> > > >>> >> -- > > >>> >> T.T.C Philips (BSc.Eng (Undergrad)) > > >>> >> Computer Science and Engineering, > > >>> >> Sri Lanka Institute of Information Technology(SLIIT) > > >>> >> > > >>> >> > > >>> >> > > >>> >> > > >>> > > > >>> > > > >>> > -- > > >>> > T.T.C Philips (BSc.Eng (Undergrad)) > > >>> > Computer Science and Engineering, > > >>> > Sri Lanka Institute of Information Technology(SLIIT) > > >>> > > >> > > >> > > >> > > >> -- > > >> T.T.C Philips (BSc.Eng (Undergrad)) > > >> Computer Science and Engineering, > > >> Sri Lanka Institute of Information Technology(SLIIT) > > >> > > >> > > >> > > >> > > > > > > > > > -- > > > T.T.C Philips (BSc.Eng (Undergrad)) > > > Computer Science and Engineering, > > > Sri Lanka Institute of Information Technology(SLIIT) > > > > > > > > > > > > > > > > > > -- > > T.T.C Philips (BSc.Eng (Undergrad)) > > Computer Science and Engineering, > > Sri Lanka Institute of Information Technology(SLIIT) > > > -- T.T.C Philips (BSc.Eng (Undergrad)) Computer Science and Engineering, Sri Lanka Institute of Information Technology(SLIIT)
