[ https://issues.apache.org/jira/browse/OFBIZ7028?page=com.atlassian.jira.plugin.system.issuetabpanels:alltabpanel
]
Jacques Le Roux updated OFBIZ7028:

Description:
After a discussion we had privately with [~gdraperi] (on PMC private ML) some time ago, I
decided to apply his recommended changes in appropriate places.
Quoting Grégory:
{quote}
I had a look to the source code and I would like to speak about how we use Random and Math.Random()
java classes.
To give a little bit of context, these two functions are marked as unsecured because if you
are able to know one output you are able to predict the future outputs.
As a short overview random java function relies on a linear congruential generator (a complex
word for a simple thing). Reference http://franklinta.com/2014/08/31/predictingthenextmathrandominjava/
LCG works like this: (a * x + c) mod m where a,c and m are static values and x is the previous
output (except for the first output where we determine x and it is called the seed)
So for example with a = 3, c = 5, m = 7 and we start with the seed x = 0,
then the next few random numbers generated will be: 5 = (3 * 0 + 5) mod 7,
6 = (3 * 5 + 5) mod 7, 2 = (3 * 6 + 5) mod 7, 4 = (3 * 2 + 5) mod 7
It means that if I know the last output (4) I can predict what will be the
next output:
(3 * 4 +5) mod 7 > 3.
We use a lot of Math.Random which relies on Random() class and also Random() class itself
especially in the LoginWorker class where it is used to generate the external link id. At
that time I have no real proven vulnerability but that worries me for two things .
# If it is possible to retrieve the state of Math.Random() from another place in the code
it will be possible to predict future links
# Even if the link is complex and almost impossible to guess by brute force we downgrade the
security as the space of possible values for the link id is really lower than the space for
session id. Example: 3EBA63DE1C0BE677A17CAB483689F9C vs EL708615493372
The topic is not urgent but I would recommend in the future using SecureRandom class and maybe
UUID.randomUUID to generate the external link id

Grégory Draperi
{quote}
Because using SecureRandom comes with a cost, I have identified the places where it's reasonable
to keep the non secured Random (like tests, internal sequences, etc.).
Using UUID.randomUUID to generate the external link id seems also the best possible solution.
Also, though there are no real proven vulnerabilities, I decided to backport as much as possible
since it's now public.
was:
After a discussion we had privately with [~gdraperi] (on PMC private ML) some time ago, I
decided to apply his recommended changes in appropriate places.
Quoting Grégory:
{quote}
I had a look to the source code and I would like to speak about how we use Random and Math.Random()
java classes.
To give a little bit of context, these two functions are marked as unsecured because if you
are able to know one output you are able to predict the future outputs.
As a short overview random java function relies on a linear congruential generator (a complex
word for a simple thing). Reference http://franklinta.com/2014/08/31/predictingthenextmathrandominjava/
LCG works like this: (a * x + c) mod m where a,c and m are static values and x is the previous
output (except for the first output where we determine x and it is called the seed)
So for example with a = 3, c = 5, m = 7 and we start with the seed x = 0,
then the next few random numbers generated will be: 5 = (3 * 0 + 5) mod 7,
6 = (3 * 5 + 5) mod 7, 2 = (3 * 6 + 5) mod 7, 4 = (3 * 2 + 5) mod 7
It means that if I know the last output (4) I can predict what will be the
next output:
(3 * 4 +5) mod 7 > 3.
We use a lot of Math.Random which relies on Random() class and also Random() class itself
especially in the LoginWorker class where it is used to generate the external link id. At
that time I have no real proven vulnerability but that worries me for two things .
# If it is possible to retrieve the state of Math.Random() from another place in the code
it will be possible to predict future links
# Even if the link is complex and almost impossible to guess by brute force we downgrade the
security as the space of possible values for the link id is really lower than the space for
session id. Example: 3EBA63DE1C0BE677A17CAB483689F9C vs EL708615493372
The topic is not urgent but I would recommend in the future using SecureRandom class and maybe
UUID.randomUUID to generate the external link id

Grégory Draperi
{quote}
Because using SecureRandom comes with a cost, I have identified the places where it's reasonnable
to keep the non secured Random (like tests, internal sequences, etc.).
Using UUID.randomUUID to generate the external link id seems also the best possible solution.
Also, though there are no real proven vulnerabilities, I decided to backport as much as possible
since it's now public.
> Use SecureRandom instead of Random where appropriate, and randomUUID for externalKey
> 
>
> Key: OFBIZ7028
> URL: https://issues.apache.org/jira/browse/OFBIZ7028
> Project: OFBiz
> Issue Type: Subtask
> Components: ALL COMPONENTS
> Affects Versions: Trunk
> Reporter: Jacques Le Roux
> Assignee: Jacques Le Roux
> Priority: Minor
>
> After a discussion we had privately with [~gdraperi] (on PMC private ML) some time ago,
I decided to apply his recommended changes in appropriate places.
> Quoting Grégory:
> {quote}
> I had a look to the source code and I would like to speak about how we use Random and
Math.Random() java classes.
> To give a little bit of context, these two functions are marked as unsecured because
if you are able to know one output you are able to predict the future outputs.
> As a short overview random java function relies on a linear congruential generator (a
complex word for a simple thing). Reference http://franklinta.com/2014/08/31/predictingthenextmathrandominjava/
> LCG works like this: (a * x + c) mod m where a,c and m are static values and x is the
previous output (except for the first output where we determine x and it is called the seed)
> So for example with a = 3, c = 5, m = 7 and we start with the seed x = 0,
> then the next few random numbers generated will be: 5 = (3 * 0 + 5) mod 7,
> 6 = (3 * 5 + 5) mod 7, 2 = (3 * 6 + 5) mod 7, 4 = (3 * 2 + 5) mod 7
> It means that if I know the last output (4) I can predict what will be the
> next output:
> (3 * 4 +5) mod 7 > 3.
> We use a lot of Math.Random which relies on Random() class and also Random() class itself
especially in the LoginWorker class where it is used to generate the external link id. At
that time I have no real proven vulnerability but that worries me for two things .
> # If it is possible to retrieve the state of Math.Random() from another place in the
code it will be possible to predict future links
> # Even if the link is complex and almost impossible to guess by brute force we downgrade
the security as the space of possible values for the link id is really lower than the space
for session id. Example: 3EBA63DE1C0BE677A17CAB483689F9C vs EL708615493372
> The topic is not urgent but I would recommend in the future using SecureRandom class
and maybe UUID.randomUUID to generate the external link id
> 
> Grégory Draperi
> {quote}
> Because using SecureRandom comes with a cost, I have identified the places where it's
reasonable to keep the non secured Random (like tests, internal sequences, etc.).
> Using UUID.randomUUID to generate the external link id seems also the best possible solution.
> Also, though there are no real proven vulnerabilities, I decided to backport as much
as possible since it's now public.

This message was sent by Atlassian JIRA
(v6.3.4#6332)
