From 1a57a83baf99e9dfa942bf38cd8874775b1f4087 Mon Sep 17 00:00:00 2001
From: Michael Kruglos <3526988+michaelkruglos@users.noreply.github.com>
Date: Thu, 28 Feb 2019 11:47:54 +0200
Subject: [PATCH] Sanitized input in publishing (#1119)

* Sanitized input in publishing

* Fixes for review

* Fixes for review

* Fixed naming for review
---
 e2e/ui/yarn.lock                                    |  3 ++-
 .../Handlers/PushHandler.cs                         | 13 ++++++++-----
 .../Handlers/ValidationHandler.cs                   |  7 +++++++
 .../Tweek.Publishing.Service.csproj                 |  2 +-
 .../Tweek.Publishing.Service/Utils/ShellHelper.cs   |  7 +++++++
 5 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/e2e/ui/yarn.lock b/e2e/ui/yarn.lock
index da429cb7f..5ff1c69bc 100644
--- a/e2e/ui/yarn.lock
+++ b/e2e/ui/yarn.lock
@@ -1942,9 +1942,10 @@ tunnel-agent@0.6.0:
   dependencies:
     safe-buffer "^5.0.1"
 
-tweek-client@^1.0.0-rc5:
+tweek-client@^1.0.0-rc7:
   version "1.0.0-rc7"
   resolved "https://registry.yarnpkg.com/tweek-client/-/tweek-client-1.0.0-rc7.tgz#af5b17cbf3dd73294829b555fbbf354cd79f563a"
+  integrity sha512-+UnxnGI7jzmUOcoa5TRW8C/U5UkJS7YCL+TErq4QNlK4ef8aFl7n03o7V4hS9fUNwcK6pNdLKGNhZ7P0hABVYA==
   dependencies:
     change-emitter "^0.1.6"
     cross-fetch "^3.0.1"
diff --git a/services/publishing/Tweek.Publishing.Service/Handlers/PushHandler.cs b/services/publishing/Tweek.Publishing.Service/Handlers/PushHandler.cs
index 34c466b02..a5a16851f 100644
--- a/services/publishing/Tweek.Publishing.Service/Handlers/PushHandler.cs
+++ b/services/publishing/Tweek.Publishing.Service/Handlers/PushHandler.cs
@@ -4,12 +4,8 @@
 using App.Metrics.Counter;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Routing;
-using Microsoft.Extensions.Logging;
-using Microsoft.Extensions.Logging.Abstractions;
-using Polly;
-using Polly.Retry;
-using Tweek.Publishing.Service.Messaging;
 using Tweek.Publishing.Service.Sync;
+using static Tweek.Publishing.Service.Utils.ShellHelper;
 
 namespace Tweek.Publishing.Service.Handlers
 {
@@ -24,6 +20,13 @@ public class PushHandler
             return async (req, res, routedata) =>
             {
                 var commitId = req.Query["commit"].ToString();
+                if (!IsCommitIdString(commitId))
+                {
+                    res.StatusCode = 400;
+                    await res.WriteAsync("Invalid commit id");
+                    return;
+                }
+
                 try
                 {
                     await syncActor.PushToUpstream(commitId);
diff --git a/services/publishing/Tweek.Publishing.Service/Handlers/ValidationHandler.cs b/services/publishing/Tweek.Publishing.Service/Handlers/ValidationHandler.cs
index 8ee84773f..233eeeb22 100644
--- a/services/publishing/Tweek.Publishing.Service/Handlers/ValidationHandler.cs
+++ b/services/publishing/Tweek.Publishing.Service/Handlers/ValidationHandler.cs
@@ -5,6 +5,7 @@
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.Extensions.Logging;
+using Tweek.Publishing.Service.Utils;
 using Tweek.Publishing.Service.Validation;
 using static Tweek.Publishing.Service.Utils.ShellHelper;
 
@@ -23,6 +24,12 @@ public class ValidationHandler
             {
                 var oldCommit = req.Query["oldrev"].ToString().Trim();
                 var newCommit = req.Query["newrev"].ToString().Trim();
+                if (!IsCommitIdString(oldCommit) || !IsCommitIdString(newCommit))
+                {
+                    res.StatusCode = 400;
+                    await res.WriteAsync("Invalid commit id");
+                    return;
+                }
 
                 var quarantinePath = req.Query["quarantinepath"].ToString();
                 var objectsDir = quarantinePath.Substring(quarantinePath.IndexOf("./objects"));
diff --git a/services/publishing/Tweek.Publishing.Service/Tweek.Publishing.Service.csproj b/services/publishing/Tweek.Publishing.Service/Tweek.Publishing.Service.csproj
index e88e20761..1a6cfd8f2 100644
--- a/services/publishing/Tweek.Publishing.Service/Tweek.Publishing.Service.csproj
+++ b/services/publishing/Tweek.Publishing.Service/Tweek.Publishing.Service.csproj
@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk.Web">
   <PropertyGroup>
-    <VersionPrefix>1.0.0-rc2</VersionPrefix>
+    <VersionPrefix>1.0.0-rc3</VersionPrefix>
     <TargetFramework>netcoreapp2.1</TargetFramework>
     <LangVersion>latest</LangVersion>
     <NoWarn>CS1998</NoWarn>
diff --git a/services/publishing/Tweek.Publishing.Service/Utils/ShellHelper.cs b/services/publishing/Tweek.Publishing.Service/Utils/ShellHelper.cs
index 2da42948b..cccddc4be 100644
--- a/services/publishing/Tweek.Publishing.Service/Utils/ShellHelper.cs
+++ b/services/publishing/Tweek.Publishing.Service/Utils/ShellHelper.cs
@@ -4,6 +4,7 @@
 using System.Reactive.Linq;
 using System.Reactive.Threading.Tasks;
 using System.Text;
+using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 
 namespace Tweek.Publishing.Service.Utils
@@ -109,6 +110,12 @@ public static (Process, Task) ExecProcess(string command, string args, Action<Pr
             });
         }
 
+        private static readonly Regex HexRegex = new Regex("^[a-f0-9]+$");
+        public static bool IsCommitIdString(string str)
+        {
+            return HexRegex.IsMatch(str);
+        }
+
         public static async Task<string> ExecTask(this ShellExecutor shellExecutor, string command, string args, Action<ProcessStartInfo> paramsInit = null)
         {
             return await shellExecutor.ExecObservable(command, args, paramsInit)