[GitHub] poi pull request #78: Add the "final" modifier to public static fields.

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] poi pull request #78: Add the "final" modifier to public static fields.

tuean
GitHub user BruceKuiLiu opened a pull request:

    https://github.com/apache/poi/pull/78

    Add the "final" modifier to public static fields.

    This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.
    http://findbugs.sourceforge.net/bugDescriptions.html#MS_SHOULD_BE_FINAL

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/BruceKuiLiu/poi master6

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/poi/pull/78.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #78
   
----
commit 6c8aa937d52f12dbc996105c89868dfaf0950c80
Author: Kui LIU <[hidden email]>
Date:   2017-10-14T18:15:34Z

    Add the "final" modifier to public static fields.
   
    This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.
    http://findbugs.sourceforge.net/bugDescriptions.html#MS_SHOULD_BE_FINAL

commit 6e57586107eb2f06db47d4bb7f5202f953ce017e
Author: Kui LIU <[hidden email]>
Date:   2017-10-14T18:19:48Z

    Add the "final" modifier to public static fields.
   
    This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.
    http://findbugs.sourceforge.net/bugDescriptions.html#MS_SHOULD_BE_FINAL

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] poi pull request #78: Add the "final" modifier to public static fields.

tuean
Github user asfgit closed the pull request at:

    https://github.com/apache/poi/pull/78


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [GitHub] poi pull request #78: Add the "final" modifier to public static fields.

moparisthebest
In reply to this post by tuean
Just wanted to point out, this only protects against accidents, it does
nothing to protect against malicious code which can just use reflection
to change even final fields.

It's still a good change, just doesn't do as advertised.

On 10/14/2017 03:05 PM, BruceKuiLiu wrote:

> GitHub user BruceKuiLiu opened a pull request:
>
>     https://github.com/apache/poi/pull/78
>
>     Add the "final" modifier to public static fields.
>
>     This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.
>     http://findbugs.sourceforge.net/bugDescriptions.html#MS_SHOULD_BE_FINAL
>
> You can merge this pull request into a Git repository by running:
>
>     $ git pull https://github.com/BruceKuiLiu/poi master6
>
> Alternatively you can review and apply these changes as the patch at:
>
>     https://github.com/apache/poi/pull/78.patch
>
> To close this pull request, make a commit to your master/trunk branch
> with (at least) the following in the commit message:
>
>     This closes #78
>    
> ----
> commit 6c8aa937d52f12dbc996105c89868dfaf0950c80
> Author: Kui LIU <[hidden email]>
> Date:   2017-10-14T18:15:34Z
>
>     Add the "final" modifier to public static fields.
>    
>     This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.
>     http://findbugs.sourceforge.net/bugDescriptions.html#MS_SHOULD_BE_FINAL
>
> commit 6e57586107eb2f06db47d4bb7f5202f953ce017e
> Author: Kui LIU <[hidden email]>
> Date:   2017-10-14T18:19:48Z
>
>     Add the "final" modifier to public static fields.
>    
>     This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.
>     http://findbugs.sourceforge.net/bugDescriptions.html#MS_SHOULD_BE_FINAL
>
> ----
>
>
> ---
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [GitHub] poi pull request #78: Add the "final" modifier to public static fields.

Dominik Stadler
I also don't think it will improve security much in normal setups, it's
just good practice to use static final for constants.

If you are concerned about something like this, you likely will use a
SecurityManager to prevent reflection, because otherwise pretty much
anything can be done via
reflection/proxying/classloading/class-shadowing/...

Dominik.

On Wed, Oct 18, 2017 at 9:31 PM, moparisthebest <[hidden email]>
wrote:

> Just wanted to point out, this only protects against accidents, it does
> nothing to protect against malicious code which can just use reflection
> to change even final fields.
>
> It's still a good change, just doesn't do as advertised.
>
> On 10/14/2017 03:05 PM, BruceKuiLiu wrote:
> > GitHub user BruceKuiLiu opened a pull request:
> >
> >     https://github.com/apache/poi/pull/78
> >
> >     Add the "final" modifier to public static fields.
> >
> >     This static field public but not final, and could be changed by
> malicious code or by accident from another package. The field could be made
> final to avoid this vulnerability.
> >     http://findbugs.sourceforge.net/bugDescriptions.html#MS_
> SHOULD_BE_FINAL
> >
> > You can merge this pull request into a Git repository by running:
> >
> >     $ git pull https://github.com/BruceKuiLiu/poi master6
> >
> > Alternatively you can review and apply these changes as the patch at:
> >
> >     https://github.com/apache/poi/pull/78.patch
> >
> > To close this pull request, make a commit to your master/trunk branch
> > with (at least) the following in the commit message:
> >
> >     This closes #78
> >
> > ----
> > commit 6c8aa937d52f12dbc996105c89868dfaf0950c80
> > Author: Kui LIU <[hidden email]>
> > Date:   2017-10-14T18:15:34Z
> >
> >     Add the "final" modifier to public static fields.
> >
> >     This static field public but not final, and could be changed by
> malicious code or by accident from another package. The field could be made
> final to avoid this vulnerability.
> >     http://findbugs.sourceforge.net/bugDescriptions.html#MS_
> SHOULD_BE_FINAL
> >
> > commit 6e57586107eb2f06db47d4bb7f5202f953ce017e
> > Author: Kui LIU <[hidden email]>
> > Date:   2017-10-14T18:19:48Z
> >
> >     Add the "final" modifier to public static fields.
> >
> >     This static field public but not final, and could be changed by
> malicious code or by accident from another package. The field could be made
> final to avoid this vulnerability.
> >     http://findbugs.sourceforge.net/bugDescriptions.html#MS_
> SHOULD_BE_FINAL
> >
> > ----
> >
> >
> > ---
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>