fix: substring bounds

aws-greengrass · Feb 5, 2024 · bdcc2d9 · bdcc2d9
1 parent 91e2ede
commit bdcc2d9
Showing 1 changed file with 30 additions and 6 deletions.
diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java b/src/main/java/com/aws/greengrass/clientdevices/auth/PermissionEvaluationUtils.java
@@ -13,6 +13,7 @@
 import com.aws.greengrass.logging.impl.LogManager;
 import com.aws.greengrass.util.Utils;
 import lombok.Builder;
+import lombok.NonNull;
 import lombok.Value;
 import org.apache.commons.lang3.StringUtils;
 
@@ -151,12 +152,12 @@ private Operation parseOperation(String operationStr) throws PolicyException {
             throw new PolicyException(EXCEPTION_MALFORMED_OPERATION);
         }
 
-        String service = operationStr.substring(0, split);
+        String service = safeSubstring(operationStr, 0, split);
         if (service.isEmpty() || !StringUtils.isAlpha(service)) {
             throw new PolicyException(EXCEPTION_MALFORMED_OPERATION);
         }
 
-        String action = operationStr.substring(split + 1);
+        String action = safeSubstring(operationStr, split + 1);
         if (action.isEmpty() || !isAlphanumericWithExtraChars(action, "-_")) {
             throw new PolicyException(EXCEPTION_MALFORMED_OPERATION);
         }
@@ -178,12 +179,12 @@ private Resource parseResource(String resourceStr) throws PolicyException  {
             throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
         }
 
-        String service = resourceStr.substring(0, split);
+        String service = safeSubstring(resourceStr, 0, split);
         if (service.isEmpty() || !StringUtils.isAlpha(service)) {
             throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
         }
 
-        String typeAndName = resourceStr.substring(split + 1);
+        String typeAndName = safeSubstring(resourceStr, split + 1);
         if (typeAndName.isEmpty()) {
             throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
         }
@@ -193,12 +194,12 @@ private Resource parseResource(String resourceStr) throws PolicyException  {
             throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
         }
 
-        String resourceType = typeAndName.substring(0, split);
+        String resourceType = safeSubstring(typeAndName, 0, split);
         if (resourceType.isEmpty() || !StringUtils.isAlpha(resourceType)) {
             throw new PolicyException(EXCEPTION_MALFORMED_RESOURCE);
         }
 
-        String resourceName = typeAndName.substring(split + 1);
+        String resourceName = safeSubstring(typeAndName, split + 1);
 
         // still using regex because Pattern.UNICODE_CHARACTER_CLASS is complicated
         if (!RESOURCE_NAME_PATTERN.matcher(resourceName).matches()) {
@@ -226,6 +227,29 @@ private static boolean isAlphanumericWithExtraChars(CharSequence cs, String extr
         return true;
     }
 
+    /**
+     * Like {@link String#substring(int, int)}, except rather than throwing, start/end
+     * indexes will be clamped to the string's length.
+     * <ul>
+     * <li>safeSubstring("a", 0, 0) -> ""</li>
+     * <li>safeSubstring("a", 0, 1) -> "a"</li>
+     * <li>safeSubstring("a", 0, 10) -> "a"</li>
+     * <li>safeSubstring("a", 1, 10) -> ""</li>
+     * </ul>
+     *
+     * @param str   input string
+     * @param start substring start, inclusive. must be greater than zero.
+     * @param end   substring end,
+     * @return substring
+     */
+    private static String safeSubstring(@NonNull String str, int start, int end) {
+        return str.substring(Math.min(start, str.length()), Math.min(end, str.length()));
+    }
+
+    private static String safeSubstring(@NonNull String str, int start) {
+        return safeSubstring(str, start, str.length());
+    }
+
     @Value
     @Builder
     private static class Operation {