Domain EE: 1821: Fix issue with possible log injection (#1928)

department-of-veterans-affairs · Aug 18, 2023 · 36590de · 36590de
1 parent 2bc40e1
commit 36590de
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 7 deletions.
diff --git a/domain-ee/ee-max-cfi-app/src/python_src/api.py b/domain-ee/ee-max-cfi-app/src/python_src/api.py
@@ -5,6 +5,7 @@
 from pydantic_models import (MaxRatingsForClaimForIncreaseRequest,
                              MaxRatingsForClaimForIncreaseResponse)
 from util.lookup_table import MAX_RATINGS_BY_CODE, get_max_rating
+from util.sanitizer import sanitize
 
 app = FastAPI(
     title="Max Ratings for CFI",
@@ -43,12 +44,12 @@ def get_health_status():
 def get_max_ratings(
         claim_for_increase: MaxRatingsForClaimForIncreaseRequest, ) -> MaxRatingsForClaimForIncreaseResponse:
     ratings = []
-    for dc in set(claim_for_increase.diagnostic_codes):
+    for dc in claim_for_increase.diagnostic_codes:
         validate_diagnostic_code(dc)
         max_rating = get_max_rating(dc)
         if max_rating:
             rating = {
-                "diagnostic_code": dc,
+                "diagnostic_code": sanitize(dc),
                 "max_rating": max_rating,
             }
             ratings.append(rating)
@@ -57,7 +58,7 @@ def get_max_ratings(
         "ratings": ratings
     }
 
-    logging.info(f"event=getMaxRating response={response}")
+    logging.info(f"event=getMaxRating ratings={ratings}")
     return response
 
 

diff --git a/domain-ee/ee-max-cfi-app/src/python_src/pydantic_models.py b/domain-ee/ee-max-cfi-app/src/python_src/pydantic_models.py
@@ -1,13 +1,13 @@
-from pydantic import BaseModel, conlist
+from pydantic import BaseModel, conint, conlist
 
 
 class MaxRatingsForClaimForIncreaseRequest(BaseModel):
-    diagnostic_codes: conlist(int, unique_items=True, min_items=1, max_items=100)
+    diagnostic_codes: conlist(conint(strict=True), unique_items=True, min_items=1, max_items=100)
 
 
 class Rating(BaseModel):
     diagnostic_code: int
-    max_rating: float
+    max_rating: int
 
 
 class MaxRatingsForClaimForIncreaseResponse(BaseModel):

diff --git a/domain-ee/ee-max-cfi-app/src/python_src/util/sanitizer.py b/domain-ee/ee-max-cfi-app/src/python_src/util/sanitizer.py
@@ -0,0 +1,2 @@
+def sanitize(obj) -> str:
+    return str(obj).replace('\r\n', '').replace('\n', '')
diff --git a/domain-ee/ee-max-cfi-app/tests/test_api.py b/domain-ee/ee-max-cfi-app/tests/test_api.py
@@ -94,10 +94,19 @@ def test_missing_params(client: TestClient):
     assert response.status_code == 422
 
 
-def test_unprocessable_content(client: TestClient):
+def test_unprocessable_content_request_does_not_have_array(client: TestClient):
     json_post_dict = {
         "diagnostic_codes": 6510,  # Should be an array
     }
 
     response = client.post(MAX_RATING, json=json_post_dict)
     assert response.status_code == 422
+
+
+def test_unprocessable_content_request_array_has_non_int_value(client: TestClient):
+    json_post_dict = {
+        "diagnostic_codes": ["6510"],  # Should be an int
+    }
+
+    response = client.post(MAX_RATING, json=json_post_dict)
+    assert response.status_code == 422