ICEfaces
  1. ICEfaces
  2. ICE-3404

IdGenerator uses dangerous random numbers

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: 1.7.1
    • Fix Version/s: None
    • Component/s: Framework
    • Labels:
      None
    • Environment:
      Java 1.5 + Tomcat 6.0
      Windows / Linux

      Description

      the IdGenerator class uses new Random() and throws away that instance.

      there is a RandomNumberGenerator but it is not used in the IdGernerator.

      we have dynamic created content (images for buttons) which are mixed up spontaneously. It seems to be related to the generated id's.



      findbugs explains this possible bug like this:

      DMI_RANDOM_USED_ONLY_ONCE: Random object created and used only once

      This code creates a java.util.Random object, uses it to generate one random number, and then discards the Random object. This produces mediocre quality random numbers and is inefficient. If possible, rewrite the code so that the Random object is created once and saved, and each time a new random number is required invoke a method on the existing Random object to obtain it.

      If it is important that the generated Random numbers not be guessable, you must not create a new Random for each random number; the values are too easily guessable. You should strongly consider using a java.security.SecureRandom instead (and avoid allocating a new SecureRandom for each random number needed).

        Activity

        Hide
        Mark Collette added a comment -

        IdGenerator seems to only be used for session ids, and the constructor that discards the Random object is not used in the code anywhere. So I don't think it's the cause of your problem. Although I can think of several ways that the session ids could be made stronger, after having looked at the code.

        If you're dynamically making image content, I assume you're using ByteArrayResource? Its calculateDigest?() method seems a little weak. You could extend that class, and override that method to use a SHA1 hash of your image data. If that alleviates your issue, we could roll that kind of fix into our code.

        Show
        Mark Collette added a comment - IdGenerator seems to only be used for session ids, and the constructor that discards the Random object is not used in the code anywhere. So I don't think it's the cause of your problem. Although I can think of several ways that the session ids could be made stronger, after having looked at the code. If you're dynamically making image content, I assume you're using ByteArrayResource? Its calculateDigest?() method seems a little weak. You could extend that class, and override that method to use a SHA1 hash of your image data. If that alleviates your issue, we could roll that kind of fix into our code.
        Hide
        Ted Goddard added a comment -

        It's important that the icefacesID is not guessable, so the random number generation for this should be examined.

        However, how does the icefacesID generation cause images to be mixed up. Is there a test case for this or sample page source that shows the collision?

        Show
        Ted Goddard added a comment - It's important that the icefacesID is not guessable, so the random number generation for this should be examined. However, how does the icefacesID generation cause images to be mixed up. Is there a test case for this or sample page source that shows the collision?
        Hide
        Werner Mueller added a comment -

        thanks for the reply.

        good question!
        at the moment it seems to be related to mixed up id's when dialogs are used. it happens in the 'background' when we use a modal popup dialog. once the dialog is shown the dynamic buttons change. it is some sort of long-shot-guess that this problem must be id related.

        the buttons are cached per session so i don't think they are exchanged from user to user. Therefore the idGenerator is probably not our delinquent.

        i'll ask our icefaces-integrators about their status too.

        Show
        Werner Mueller added a comment - thanks for the reply. good question! at the moment it seems to be related to mixed up id's when dialogs are used. it happens in the 'background' when we use a modal popup dialog. once the dialog is shown the dynamic buttons change. it is some sort of long-shot-guess that this problem must be id related. the buttons are cached per session so i don't think they are exchanged from user to user. Therefore the idGenerator is probably not our delinquent. i'll ask our icefaces-integrators about their status too.
        Hide
        Stefan Niederhauser added a comment -

        Hi Mark, hi Ted

        I don't think it has to do with the dynamically created images as not only the images are mixed up, but also the actions behind it.
        I suspect it's something with the source tags of icefaces and the ajax of icefaces that causes the problems.

        The problem is that some images are mixed up when the user opens a panelPopup with dynamic contents.
        Interestingly, this happens only the first time the user is on that page. As soon as he reloads the page or goes to another page and comes back, everything works fine.
        The problem reappears only when the user opens a new session.

        Show
        Stefan Niederhauser added a comment - Hi Mark, hi Ted I don't think it has to do with the dynamically created images as not only the images are mixed up, but also the actions behind it. I suspect it's something with the source tags of icefaces and the ajax of icefaces that causes the problems. The problem is that some images are mixed up when the user opens a panelPopup with dynamic contents. Interestingly, this happens only the first time the user is on that page. As soon as he reloads the page or goes to another page and comes back, everything works fine. The problem reappears only when the user opens a new session.
        Show
        Stefan Niederhauser added a comment - see also forum: http://www.icefaces.org/JForum/posts/list/0/9188.page
        Hide
        Mark Collette added a comment -

        No longer relevant with ICEfaces 2+

        Show
        Mark Collette added a comment - No longer relevant with ICEfaces 2+

          People

          • Assignee:
            Unassigned
            Reporter:
            Werner Mueller
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: