From f1f01906979b5baf1b11f1b6849b05e4642aabfc Mon Sep 17 00:00:00 2001
From: Pamela Dragosh <pdragosh@research.att.com>
Date: Fri, 15 Jan 2021 09:59:27 -0500
Subject: Remove unused import and add comments

Removes unused import and also adds a check for file size.
Since these entries are opened in memory, use NOSONAR to clear
sonar security hotspot.

Issue-ID: POLICY-2908
Change-Id: Ic3511a3f59cd2d78301316df209de5da1e25acdb
Signed-off-by: Pamela Dragosh <pdragosh@research.att.com>
---
 .../file/PolicyDecoderFileInCsarToPolicy.java      | 34 +++++++++++++++++-----
 1 file changed, 26 insertions(+), 8 deletions(-)

(limited to 'plugins/reception-plugins/src/main/java')

diff --git a/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/PolicyDecoderFileInCsarToPolicy.java b/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/PolicyDecoderFileInCsarToPolicy.java
index 282578d0..1e04b932 100644
--- a/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/PolicyDecoderFileInCsarToPolicy.java
+++ b/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/PolicyDecoderFileInCsarToPolicy.java
@@ -24,7 +24,6 @@ package org.onap.policy.distribution.reception.decoding.policy.file;
 
 import java.io.IOException;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Enumeration;
@@ -49,6 +48,7 @@ public class PolicyDecoderFileInCsarToPolicy implements PolicyDecoder<Csar, Tosc
 
     private PolicyDecoderFileInCsarToPolicyParameterGroup decoderParameters;
     private StandardCoder coder;
+    private static final long MAX_FILE_SIZE = 512 * 1024;
 
     /**
      * {@inheritDoc}.
@@ -77,8 +77,13 @@ public class PolicyDecoderFileInCsarToPolicy implements PolicyDecoder<Csar, Tosc
         try (ZipFile zipFile = new ZipFile(csar.getCsarPath())) {
             final Enumeration<? extends ZipEntry> entries = zipFile.entries();
             while (entries.hasMoreElements()) {
-                final ZipEntry entry = entries.nextElement();
-                if (isZipEntryValid(entry, csar.getCsarPath())) {
+                //
+                // Sonar will flag this as a Security Hotspot
+                // "Expanding archive files is security-sensitive"
+                // isZipEntryValid ensures the file being read exists in the archive
+                //
+                final ZipEntry entry = entries.nextElement(); // NOSONAR
+                if (isZipEntryValid(entry.getName(), csar.getCsarPath(), entry.getSize())) {
                     final ToscaServiceTemplate policy =
                             coder.decode(zipFile.getInputStream(entry), ToscaServiceTemplate.class);
                     policyList.add(policy);
@@ -99,18 +104,31 @@ public class PolicyDecoderFileInCsarToPolicy implements PolicyDecoder<Csar, Tosc
      * @param entry the ZipEntry to check
      * @param csarPath Absolute path to the csar the ZipEntry is in
      * @return true if no injection detected, and it is a policy type  or policy file.
+     * @throws PolicyDecodingException if the file size is too large
      */
-    private boolean isZipEntryValid(ZipEntry entry, String csarPath) {
+    private boolean isZipEntryValid(String entryName, String csarPath, long entrySize) throws PolicyDecodingException {
         //
         // We only care about policy types and policies
         //
-        if (entry.getName().contains(decoderParameters.getPolicyTypeFileName())
-                || entry.getName().contains(decoderParameters.getPolicyFileName())) {
+        if (entryName.contains(decoderParameters.getPolicyTypeFileName())
+                || entryName.contains(decoderParameters.getPolicyFileName())) {
+            //
+            // Check file size
+            //
+            if (entrySize > MAX_FILE_SIZE) {
+                throw new PolicyDecodingException("Zip entry for " + entryName + " is too large " + entrySize);
+            }
             //
             // Now ensure that there is no path injection
             //
-            Path path = Path.of(csarPath, entry.getName()).normalize();
-            return path.startsWith(csarPath);
+            Path path = Path.of(csarPath, entryName).normalize();
+            //
+            // Throw an exception if path is outside the csar
+            //
+            if (! path.startsWith(csarPath)) {
+                throw new PolicyDecodingException("Potential path injection for zip entry " + entryName);
+            }
+            return true;
         }
 
         return false;
-- 
cgit 1.2.3-korg