fineract-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thisura Philips <ttcphil...@gmail.com>
Subject Re: [GSOC2017] Fixing security vulnerabilities reported by sonar scan
Date Wed, 10 May 2017 19:07:23 GMT
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 <ttcphilips@gmail.com>
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/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/1uLk3YPcjnXk7RqF8etsTzIuN59CDU
> 6sgBxpZul__1V4/
>
> Thanks & Regards
>
> On Sat, Apr 29, 2017 at 8:12 AM, Thisura Philips <ttcphilips@gmail.com>
> 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 <myrle@apache.org> 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 <ttcphilips@gmail.com>
>>> 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 <ttcphilips@gmail.com
>>> >
>>> > 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 <
>>> ttcphilips@gmail.com>
>>> >> 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)

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message