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 Sun, 30 Apr 2017 02:11:30 GMT
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 <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)

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