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/1uLk3YPcjnXk7RqF8etsTzIuN59CDU6sgBxpZul__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/1uLk3YPcjnXk7RqF8etsTzIuN59CDU6sgBxpZul__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.ProvisioningEntriesDefinitionJsonDeserializer (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)
