fineract-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ed Cable <edca...@mifos.org>
Subject Re: [GSOC2017] Fixing security vulnerabilities reported by sonar scan
Date Mon, 29 May 2017 23:43:32 GMT
Adding Nazeer too.

Ed

On May 27, 2017 9:43 PM, "Thisura Philips" <ttcphilips@gmail.com> 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 <edcable@mifos.org> wrote:
>
>> Thisura, I have cc'd Mark on this email
>>
>> On May 10, 2017 14:07, "Thisura Philips" <ttcphilips@gmail.com> 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 <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/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 <
>> 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.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 <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/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 <
>> > 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)
>> >
>>
>
>
>
> --
> 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