Mailing-List: contact dev-help@fineract.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@fineract.apache.org
MIME-Version: 1.0
In-Reply-To: <CAGGKnDA1j-KGFFRnD6PKDdAfHMKoe3BZwCNXOPBdm0RtU1o4MQ@mail.gmail.com>
References: <CAGGKnDBtC0eT948J71-0eU+qV9xCvCjQnmKc49L6feyuM8RgQg@mail.gmail.com>
 <CAGGKnDA04ETv2m67pyNDhkxxYTXSPUjdLK6Hbp9ogAms042zcg@mail.gmail.com> <CAGGKnDA1j-KGFFRnD6PKDdAfHMKoe3BZwCNXOPBdm0RtU1o4MQ@mail.gmail.com>
From: Myrle Krantz <myrle@apache.org>
Date: Fri, 28 Apr 2017 14:19:39 +0200
Message-ID: <CAJdZ4O3wB7848xn3jMZLs9RT52F-1aweuSkhyyPZChP+fTb5jQ@mail.gmail.com>
Subject: Re: [GSOC2017] Fixing security vulnerabilities reported by sonar scan
To: dev@fineract.apache.org
Content-Type: text/plain; charset=UTF-8
archived-at: Fri, 28 Apr 2017 12:19:43 -0000

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.ProvisioningEntriesApiConstants.
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=1uLk3YPcjnXk7RqF8etsTzIuN59CDU6sgBxpZul__1V4>
> .
>
> Is there any particular reason to
> have org.apache.fineract.accounting.provisioning.constant.ProvisioningEntriesApiConstants
> 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=1TdwwHM2K1gMb6qILEX7gmzU8dVXcHGBdh569aFJfB2U>
>>> [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=1uLk3YPcjnXk7RqF8etsTzIuN59CDU6sgBxpZul__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=1uLk3YPcjnXk7RqF8etsTzIuN59CDU6sgBxpZul__1V4>)
>>> and solutions that I have suggested in the summary [3]
>>> <https://drive.google.com/open?id=1TdwwHM2K1gMb6qILEX7gmzU8dVXcHGBdh569aFJfB2U>
>>> .
>>>
>>> According to the findings [4]
>>> <https://drive.google.com/open?id=1uLk3YPcjnXk7RqF8etsTzIuN59CDU6sgBxpZul__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)