diff --git a/deploy/docker/docker-compose.yml b/deploy/docker/docker-compose.yml index 97660701..65c9a2bb 100755 --- a/deploy/docker/docker-compose.yml +++ b/deploy/docker/docker-compose.yml @@ -120,6 +120,7 @@ services: - DB_HOST=postgresdb - DB_PORT=5432 - SERVER_PORT=${WORKSHOP_SERVER_PORT:-8000} + - ENABLE_SHELL_INJECTION=${ENABLE_SHELL_INJECTION:-false} - MONGO_DB_HOST=mongodb - MONGO_DB_PORT=27017 - MONGO_DB_USER=admin diff --git a/deploy/helm/templates/workshop/config.yaml b/deploy/helm/templates/workshop/config.yaml index 6c722735..b44cc641 100644 --- a/deploy/helm/templates/workshop/config.yaml +++ b/deploy/helm/templates/workshop/config.yaml @@ -20,6 +20,7 @@ data: MONGO_DB_PASSWORD: {{ .Values.mongodb.config.mongoPassword }} MONGO_DB_NAME: {{ .Values.mongodb.config.mongoDbName }} SERVER_PORT: {{ .Values.workshop.port | quote }} + ENABLE_SHELL_INJECTION: {{ .Values.enableShellInjection | quote }} API_GATEWAY_URL: {{ if .Values.apiGatewayServiceInstall }}"https://{{ .Values.apiGatewayService.service.name }}"{{ else }}{{ .Values.apiGatewayServiceUrl }}{{ end }} TLS_ENABLED: {{ .Values.tlsEnabled | quote }} FILES_LIMIT: {{ .Values.workshop.config.filesLimit | quote }} diff --git a/deploy/k8s/base/workshop/config.yaml b/deploy/k8s/base/workshop/config.yaml index b10d7e90..45719e7a 100644 --- a/deploy/k8s/base/workshop/config.yaml +++ b/deploy/k8s/base/workshop/config.yaml @@ -18,6 +18,7 @@ data: MONGO_DB_PASSWORD: crapisecretpassword MONGO_DB_NAME: crapi SERVER_PORT: "8000" + ENABLE_SHELL_INJECTION: "false" API_GATEWAY_URL: "https://api.mypremiumdealership.com" # Gunicorn configuration for better performance under load GUNICORN_WORKERS: "4" diff --git a/docs/challengeSolutions.md b/docs/challengeSolutions.md index c512a623..6f4dc659 100644 --- a/docs/challengeSolutions.md +++ b/docs/challengeSolutions.md @@ -130,4 +130,17 @@ The above challenge was completed using Burp Suite Community Edition. - `AA==` is the Base64 encoded form of Hex null byte `00` - This JWT will be accepted as a valid JWT Token by crAPI +## Command Injection + +### [Challenge 19 - Execute an additional command through the Workshop diagnostic flow](challenges.md#challenge-19---execute-an-additional-command-through-the-workshop-diagnostic-flow) + +#### Detailed solution + +1. Login to crAPI and add a vehicle. +2. Start the Workshop service with `ENABLE_SHELL_INJECTION=true` in the lab environment. +3. Open the *Contact Mechanic* flow and observe the request to `/workshop/api/merchant/contact_mechanic`. +4. Add `diagnostic_command` to the request body with a normal value such as `status`. +5. Change `diagnostic_command` to `status; echo crapi-command-injection`. +6. Send the request and confirm that `diagnostic_output` contains `crapi-command-injection`. + ## << 2 secret challenges >> diff --git a/docs/challenges.md b/docs/challenges.md index 85745289..a118eb03 100644 --- a/docs/challenges.md +++ b/docs/challenges.md @@ -113,6 +113,20 @@ Extract the credentials of another user and check their orders. Use the chatbot to perform an action like placing order on behalf on another user. +## Command Injection + +### Challenge 19 - Execute an additional command through the Workshop diagnostic flow + +crAPI lets vehicle owners contact a mechanic and send vehicle problem details to the Workshop service. When `ENABLE_SHELL_INJECTION=true` is set for the Workshop service, the report submission flow also accepts a diagnostic command that is passed to a shell command in the Workshop container. + +* Analyze the request sent by the contact mechanic flow. + +* Enable the lab-only shell injection behavior with `ENABLE_SHELL_INJECTION=true`. + +* Add a command separator to the diagnostic command. + +* Confirm that the response includes output from the additional command. + ## << 3 secret challenges >> There are two more secret challenges in crAPI, that are pretty complex, and for now we don’t share details about them, except the fact they are really cool. diff --git a/openapi-spec/crapi-openapi-spec.json b/openapi-spec/crapi-openapi-spec.json index c093c11d..271943fc 100644 --- a/openapi-spec/crapi-openapi-spec.json +++ b/openapi-spec/crapi-openapi-spec.json @@ -2408,6 +2408,10 @@ }, "mechanic_code" : { "type" : "string" + }, + "diagnostic_command" : { + "type" : "string", + "description" : "Lab-only diagnostic command. Executed only when ENABLE_SHELL_INJECTION=true is set for the Workshop service." } } }, @@ -2417,7 +2421,8 @@ "number_of_repeats" : 1, "repeat_request_if_failed" : false, "problem_details" : "Hi Jhon", - "vin" : "8UOLV89RGKL908077" + "vin" : "8UOLV89RGKL908077", + "diagnostic_command" : "status" } } } @@ -2444,6 +2449,10 @@ }, "report_link" : { "type" : "string" + }, + "diagnostic_output" : { + "type" : "string", + "description" : "Returned only when ENABLE_SHELL_INJECTION=true is set for the Workshop service." } } }, @@ -2457,7 +2466,8 @@ "response_from_mechanic_api" : { "id" : 17, "sent" : true, - "report_link" : "http://localhost:8888/workshop/api/mechanic/mechanic_report?report_id=17" + "report_link" : "http://localhost:8888/workshop/api/mechanic/mechanic_report?report_id=17", + "diagnostic_output" : "Running diagnostic: status\n" }, "status" : 200 } @@ -2531,6 +2541,15 @@ "type" : "string", "example" : "0BZCX25UTBJ987271" } + }, { + "name" : "diagnostic_command", + "in" : "query", + "required" : false, + "schema" : { + "type" : "string", + "example" : "status", + "description" : "Lab-only diagnostic command. Executed only when ENABLE_SHELL_INJECTION=true is set for the Workshop service." + } } ], "responses" : { "200" : { @@ -2550,6 +2569,10 @@ "report_link" : { "type" : "string", "format" : "url" + }, + "diagnostic_output" : { + "type" : "string", + "description" : "Returned only when ENABLE_SHELL_INJECTION=true is set for the Workshop service." } } } diff --git a/services/chatbot/src/resources/crapi-openapi-spec.json b/services/chatbot/src/resources/crapi-openapi-spec.json index 9934a04e..ac4bb492 100644 --- a/services/chatbot/src/resources/crapi-openapi-spec.json +++ b/services/chatbot/src/resources/crapi-openapi-spec.json @@ -3927,6 +3927,10 @@ }, "mechanic_code": { "type": "string" + }, + "diagnostic_command": { + "type": "string", + "description": "Lab-only diagnostic command. Executed only when ENABLE_SHELL_INJECTION=true is set for the Workshop service." } } }, @@ -3936,7 +3940,8 @@ "number_of_repeats": 1, "repeat_request_if_failed": false, "problem_details": "Hi Jhon", - "vin": "8UOLV89RGKL908077" + "vin": "8UOLV89RGKL908077", + "diagnostic_command": "status" } } } @@ -3970,6 +3975,10 @@ }, "report_link": { "type": "string" + }, + "diagnostic_output": { + "type": "string", + "description": "Returned only when ENABLE_SHELL_INJECTION=true is set for the Workshop service." } } }, @@ -3983,7 +3992,8 @@ "response_from_mechanic_api": { "id": 17, "sent": true, - "report_link": "http://localhost:8888/workshop/api/mechanic/mechanic_report?report_id=17" + "report_link": "http://localhost:8888/workshop/api/mechanic/mechanic_report?report_id=17", + "diagnostic_output": "Running diagnostic: status\n" }, "status": 200 } @@ -4066,6 +4076,16 @@ "type": "string", "example": "0BZCX25UTBJ987271" } + }, + { + "name": "diagnostic_command", + "in": "query", + "required": false, + "schema": { + "type": "string", + "example": "status", + "description": "Lab-only diagnostic command. Executed only when ENABLE_SHELL_INJECTION=true is set for the Workshop service." + } } ], "responses": { @@ -4090,6 +4110,10 @@ "report_link": { "type": "string", "format": "url" + }, + "diagnostic_output": { + "type": "string", + "description": "Returned only when ENABLE_SHELL_INJECTION=true is set for the Workshop service." } } } @@ -5302,4 +5326,4 @@ } } } -} \ No newline at end of file +} diff --git a/services/workshop/crapi/mechanic/serializers.py b/services/workshop/crapi/mechanic/serializers.py index 0ee38a4d..5b076151 100644 --- a/services/workshop/crapi/mechanic/serializers.py +++ b/services/workshop/crapi/mechanic/serializers.py @@ -81,6 +81,7 @@ class ReceiveReportSerializer(serializers.Serializer): problem_details = serializers.CharField() vin = serializers.CharField() owner_id = serializers.CharField(required=False) + diagnostic_command = serializers.CharField(required=False) class SignUpSerializer(serializers.Serializer): diff --git a/services/workshop/crapi/mechanic/tests.py b/services/workshop/crapi/mechanic/tests.py index 8fb506d5..1f00c6d5 100644 --- a/services/workshop/crapi/mechanic/tests.py +++ b/services/workshop/crapi/mechanic/tests.py @@ -13,8 +13,10 @@ """ contains all the test cases related to mechanic """ -from django.utils import timezone +import os from unittest.mock import patch + +from django.utils import timezone from utils.mock_methods import ( get_sample_mechanic_data, mock_jwt_auth_required, @@ -254,6 +256,60 @@ def setUp(self): updated_on=timezone.now(), ) + @patch.dict(os.environ, {"ENABLE_SHELL_INJECTION": "false"}) + def test_receive_report_diagnostic_disabled_without_flag(self): + """ + diagnostic commands are ignored unless shell injection is enabled + :return: None + """ + payload = dict(self.contact_mechanic_request_body) + payload["diagnostic_command"] = "status; echo crapi-command-injection" + res = self.client.get( + "/workshop/api/mechanic/receive_report", + payload, + **self.user_auth_headers, + content_type="application/json", + ) + self.assertEqual(res.status_code, 200) + self.assertNotIn("diagnostic_output", res.json()) + + @patch.dict(os.environ, {"ENABLE_SHELL_INJECTION": "true"}) + def test_receive_report_diagnostic_normal_input(self): + """ + normal diagnostic input is echoed in the report response + :return: None + """ + payload = dict(self.contact_mechanic_request_body) + payload["diagnostic_command"] = "status" + res = self.client.get( + "/workshop/api/mechanic/receive_report", + payload, + **self.user_auth_headers, + content_type="application/json", + ) + self.assertEqual(res.status_code, 200) + self.assertIn("diagnostic_output", res.json()) + self.assertIn("Running diagnostic: status", res.json()["diagnostic_output"]) + + @patch.dict(os.environ, {"ENABLE_SHELL_INJECTION": "true"}) + def test_receive_report_diagnostic_command_injection(self): + """ + a command separator executes the injected diagnostic command + :return: None + """ + payload = dict(self.contact_mechanic_request_body) + payload["diagnostic_command"] = "status; echo crapi-command-injection" + res = self.client.get( + "/workshop/api/mechanic/receive_report", + payload, + **self.user_auth_headers, + content_type="application/json", + ) + self.assertEqual(res.status_code, 200) + output_lines = res.json()["diagnostic_output"].splitlines() + self.assertIn("Running diagnostic: status", output_lines) + self.assertIn("crapi-command-injection", output_lines) + def test_create_comment(self): """ creates a dummy service request diff --git a/services/workshop/crapi/mechanic/views.py b/services/workshop/crapi/mechanic/views.py index 684cdd63..c4dc48cd 100644 --- a/services/workshop/crapi/mechanic/views.py +++ b/services/workshop/crapi/mechanic/views.py @@ -18,6 +18,7 @@ import os import bcrypt import re +import subprocess from urllib.parse import unquote from django.template.loader import get_template from xhtml2pdf import pisa @@ -46,6 +47,7 @@ ) from rest_framework.pagination import LimitOffsetPagination + class SignUpView(APIView): """ Used to add a new mechanic @@ -199,10 +201,15 @@ def get(self, request): reverse("get-mechanic-report"), service_request.id ) report_link = request.build_absolute_uri(report_link) - return Response( - {"id": service_request.id, "sent": True, "report_link": report_link}, - status=status.HTTP_200_OK, - ) + response_data = { + "id": service_request.id, + "sent": True, + "report_link": report_link, + } + diagnostic_command = report_details.get("diagnostic_command") + if diagnostic_command and is_shell_injection_enabled(): + response_data["diagnostic_output"] = run_diagnostic(diagnostic_command) + return Response(response_data, status=status.HTTP_200_OK) class GetReportView(APIView): @@ -415,6 +422,31 @@ def validate_filename(input: str) -> bool: return bool(url_encoded_pattern.fullmatch(input)) +def is_shell_injection_enabled() -> bool: + return os.environ.get("ENABLE_SHELL_INJECTION", "").lower() == "true" + + +def run_diagnostic(command: str) -> str: + """ + Shells out with user input on purpose for the command injection challenge. + """ + try: + completed_process = subprocess.run( + f"echo Running diagnostic: {command}", + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + shell=True, + text=True, + timeout=5, + ) + return completed_process.stdout + except subprocess.TimeoutExpired as exception: + output = exception.stdout or "" + if isinstance(output, bytes): + output = output.decode(errors="replace") + return f"{output}\nDiagnostic command timed out." + + def service_report_pdf(response_data, report_id): """ Generates service report's PDF file from a template and saves it to the disk. @@ -457,4 +489,4 @@ def manage_reports_directory(): os.remove(oldest_file) except (OSError, FileNotFoundError) as e: - print(f"Error during report directory management: {e}") \ No newline at end of file + print(f"Error during report directory management: {e}") diff --git a/services/workshop/crapi/merchant/serializers.py b/services/workshop/crapi/merchant/serializers.py index 00c2f2f7..7168974b 100644 --- a/services/workshop/crapi/merchant/serializers.py +++ b/services/workshop/crapi/merchant/serializers.py @@ -28,6 +28,7 @@ class ContactMechanicSerializer(serializers.Serializer): mechanic_api = serializers.CharField() repeat_request_if_failed = serializers.BooleanField(required=False) number_of_repeats = serializers.IntegerField(required=False) + diagnostic_command = serializers.CharField(required=False) class MechanicPublicSerializer(serializers.ModelSerializer): diff --git a/services/workshop/crapi/merchant/tests.py b/services/workshop/crapi/merchant/tests.py index 7a937947..d27417e8 100644 --- a/services/workshop/crapi/merchant/tests.py +++ b/services/workshop/crapi/merchant/tests.py @@ -13,7 +13,7 @@ """ contains all the test cases related to merchant """ -from unittest.mock import patch +from unittest.mock import Mock, patch from utils.mock_methods import ( get_sample_mechanic_data, mock_jwt_auth_required, @@ -164,6 +164,42 @@ def test_repeat_missing_request(self): ) self.assertEqual(res.status_code, 200) + @patch("crapi.merchant.views.requests.get") + def test_contact_mechanic_forwards_diagnostic_command(self, mocked_get): + """ + diagnostic_command is forwarded to the mechanic api request + :return: None + """ + mechanic_response = Mock() + mechanic_response.status_code = 200 + mechanic_response.json.return_value = { + "id": 1, + "sent": True, + "report_link": ( + "http://localhost:8888/workshop/api/mechanic/" + "mechanic_report?report_id=1" + ), + "diagnostic_output": "Running diagnostic: status\n", + } + mocked_get.return_value = mechanic_response + + self.contact_mechanic_request_body["diagnostic_command"] = "status" + res = self.client.post( + "/workshop/api/merchant/contact_mechanic", + self.contact_mechanic_request_body, + **self.user_auth_headers, + content_type="application/json", + ) + + self.assertEqual(res.status_code, 200) + self.assertEqual( + res.json()["response_from_mechanic_api"]["diagnostic_output"], + "Running diagnostic: status\n", + ) + self.assertEqual( + mocked_get.call_args.kwargs["params"]["diagnostic_command"], "status" + ) + def test_receive_report_and_get_report(self): """ tests receive_report with a valid request