Uploaded image for project: 'SonarJava'
  1. SonarJava
  2. SONARJAVA-3532

S5042 should focus on zipbomb attacks

    XMLWordPrintable

    Details

    • Type: False-Positive
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 6.13
    • Component/s: Rules
    • Labels:

      Description

      Currently the rule raises on each occurrence of java.util.zip.ZipEntry or org.apache.commons.compress.archivers.ArchiveEntry kind of objects, for instance just the declaration of an object is sensitive:

        void foo(
          ZipEntry zipEntry, // Sensitive
      

      The rule S5042 has been updated to focus only on "Zipbomb attacks" which occur when the developer doesn't control resource consumption when expanding archives, like below:

      File f = new File("ZipBomb.zip");
      ZipFile zipFile = new ZipFile(f);
      Enumeration<? extends ZipEntry> entries = zipFile.entries();  // Sensitive
         
      while(entries.hasMoreElements()) { 
        ZipEntry ze = entries.nextElement(); 
        File out = new File("./output_onlyfortesting.txt");
        Files.copy(zipFile.getInputStream(ze), out.toPath(), StandardCopyOption.REPLACE_EXISTING);
      }
      

      We should raise in three cases:

      With the following exception:
      In java, most of the fixes for this vulnerability rely on reading the inputstream of a zipentry and to compute metrics to see if the entry is malicious or not (zipentry.getsize() isnot reliable because zipentry header size can be forged by attackers):

      File f = new File("ZipBomb.zip");
      ZipFile zipFile = new ZipFile(f);
      Enumeration<? extends ZipEntry> entries = zipFile.entries();
      
      int THRESHOLD_ENTRIES = 10000;        
      int THRESHOLD_SIZE = 1000000000; // 1 GB
      double THRESHOLD_RATIO = 10;         
      int totalSizeArchive = 0;
      int totalEntryArchive = 0;
              
      while(entries.hasMoreElements()) { 
        ZipEntry ze = entries.nextElement();
        InputStream in = new BufferedInputStream(zipFile.getInputStream(ze)); 
        OutputStream out = new BufferedOutputStream(new FileOutputStream("./output_onlyfortesting.txt"));
                
        totalEntryArchive ++;
      
        int nBytes = -1;
        byte[] buffer = new byte[2048];
        int totalSizeEntry = 0;
                
        while((nBytes = in.read(buffer)) > 0) { // Compliant
            out.write(buffer, 0, nBytes);
            totalSizeEntry += nBytes;
            totalSizeArchive += nBytes; 
                  
            double compressionRatio = totalSizeEntry / ze.getCompressedSize();
            if(compressionRatio > THRESHOLD_RATIO) {
              // ratio between compressed and uncompressed data is highly suspicious, looks like a Zip Bomb Attack
              break;
            }
        }
      
        if(totalSizeArchive > THRESHOLD_SIZE) {
            // the uncompressed data size is too much for the application resource capacity
            break;
        }
          
        if(totalEntryArchive > THRESHOLD_ENTRIES) {
            // too much entries in this archive, can lead to inodes exhaustion of the system
            break;
        } 
      }
      

      So we should look in the same function if the call to in.read( is done, if yes, likely the developer will read the inputstream to control the decompression, then no issue should be raised.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              sebastian.hungerecker Sebastian Hungerecker
              Reporter:
              eric.therond Eric Therond
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Due:
                Created:
                Updated:
                Resolved: