RandomAccessReferenceMap.update() can randomly corrupt the map

[meg23]

From cyounk...@gmail.com on August 06, 2009 15:33:16

What steps will reproduce the problem? You have a ARM set up like this:
direct1 <=> "AAAA"

You do this:
newSet = new Set()
newSet.add(newDirect)
newSet.add(direct1)
ARM.update(newSet)

update() clears the old mappings, but tries to preserve them if the same
direct objects are referenced in the new set. Here's how this could go awry:

• update() clears both maps
• update() can't find newDirect in the old mapping, so it calls
  getUniqueRandomReference(), which randomly returns "AAAA". It checks that
  "AAAA" is not in the current mapping, and it's not (It was just cleared).
• update() sets the mapping newDirect <=> "AAAA"
• update() then looks at direct1, sees that it is in the old mapping, and
  sets it in the new mapping direct1 <=> "AAAA", corrupting the indirect to
  direct mapping which is now "AAAA" -> direct1

How to fix it:

• Change getUniqueRandomReference to take in a list of invalid indirect
  keys and ensure the generated one is not in that list
• Preserve the old indirect to direct map in update()
• Pass in the keys of the old indirect to direct map to
  getUniqueRandomReference What version of the product are you using? On what operating system? SVN revision 574 .

Original issue: http://code.google.com/p/owasp-esapi-java/issues/detail?id=27

[meg23]

From chrisisbeef on December 01, 2009 23:28:24

I believe, and correct me if I am wrong here, that the chances of this happening are
so far fetched that the amount of work to mitigate this nullify the benefit of going
through this work.

Also, with how impropable this situation is, is it worth affecting the performance of
an otherwise performant object?

[meg23]

From chrisisbeef on December 01, 2009 23:55:02

Labels: -Priority-Medium Priority-Low

[meg23]

From cyounk...@gmail.com on December 02, 2009 15:46:58

Well, I guess it depends on whether or not you want this thing to actually work reliably.

These are unchecked insertions, so the probability of a collision is the # of
elements in the map / the keyspace of the map keys. Multiply this by the number of
insertions to get roughly the probability of a collision on an update(). It's small
problems like this that are extremely elusive and hard to track down.

In the report I stated how to fix it. It would take at most 10 minutes to change and
test.

[meg23]

From manico.james@gmail.com on October 31, 2010 22:59:29

Younkins is right IMO. I'll fix this before 2.0.

Owner: manico.james
Labels: -Priority-Low Priority-High Milestone-Release2.0

[meg23]

From manico.james@gmail.com on October 31, 2010 23:01:43

Status: Accepted

[meg23]

From chrisisbeef on November 20, 2010 13:16:56

Labels: Component-Other

[meg23]

From manico.james@gmail.com on May 28, 2012 20:20:12

Owner: chrisisbeef

[kwwall]

Closed via PR #469