Adding Nazeer too. Ed
On May 27, 2017 9:43 PM, "Thisura Philips" <[email protected]> wrote: > 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/1uLk3YPcjnXk7RqF8ets >> TzIuN59CDU >> > 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/def >> initions/582.html> >> > - >> > >>> > Array Declared Public, Final, and Static >> > >>> > - MITRE, CWE-607 <http://cwe.mitre.org/data/def >> initions/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) > > > >
