Sakai
  1. Sakai
  2. SAK-19081 2.8 Static code review sweep
  3. SAK-19854

Incorrect synchronization in Resources tool (content.tool)

    Details

    • Type: Sub-task Sub-task
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: No Resources
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.7.3 [tentative]
    • Component/s: Content
    • Labels:
      None
    • Environment:
      Static code review Findbugs for 2.8_b3

      Description

      Code review signals the following piece of code as an issue. - Synchronization on boxed primitive

      https://source.sakaiproject.org/svn/content/trunk/content-tool/tool/src/java/org/sakaiproject/content/tool/ResourcesBrowseItem.java line 127

      More info at: https://www.securecoding.cert.org/confluence/display/java/CON08-J.+Do+not+call+alien+methods+that+synchronize+on+the+same+objects+as+any+callers+in+the+execution+chain

      public ResourcesBrowseItem(String id, String name, String type)
      {
      m_name = name;
      m_id = id;
      m_type = type;

      Integer snum;
      synchronized(seqnum)
      {
      snum = seqnum;
      seqnum = Integer.valueOf((seqnum.intValue() + 1) % 10000);
      }

      I can see 2 issues:

      1) The reference to the lock is changing while locked
      seqnum = Integer.valueOf((seqnum.intValue() + 1) % 10000);

      2) Integer.valueOf is a cached value potnetially used elsewhere.

        Activity

        There are no comments yet on this issue.

          People

          • Assignee:
            Unassigned
            Reporter:
            Alan Berg
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: