From 70274bea91ee3789b495a0ebe353e4309da8cfd9 Mon Sep 17 00:00:00 2001 From: Ti Chi Robot Date: Fri, 13 Aug 2021 15:33:17 +0800 Subject: [PATCH] api: restrict persist-file to only accept JSON data (#3969) (#3973) Signed-off-by: disksing Co-authored-by: disksing --- server/api/admin.go | 7 ++++++- server/api/admin_test.go | 9 +++++++++ server/server.go | 1 + 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/server/api/admin.go b/server/api/admin.go index a3676c560f9..8f14e99d2bb 100644 --- a/server/api/admin.go +++ b/server/api/admin.go @@ -14,6 +14,7 @@ package api import ( + "encoding/json" "io" "net/http" "strconv" @@ -95,7 +96,7 @@ func (h *adminHandler) ResetTS(w http.ResponseWriter, r *http.Request) { } // Intentionally no swagger mark as it is supposed to be only used in -// server-to-server. +// server-to-server. For security reason, it only accepts JSON formatted data. func (h *adminHandler) persistFile(w http.ResponseWriter, r *http.Request) { data, err := io.ReadAll(r.Body) if err != nil { @@ -103,6 +104,10 @@ func (h *adminHandler) persistFile(w http.ResponseWriter, r *http.Request) { return } defer r.Body.Close() + if !json.Valid(data) { + h.rd.Text(w, http.StatusBadRequest, "body should be json format") + return + } err = h.svr.PersistFile(mux.Vars(r)["file_name"], data) if err != nil { h.rd.Text(w, http.StatusInternalServerError, err.Error()) diff --git a/server/api/admin_test.go b/server/api/admin_test.go index 497d3055740..889f0cc375d 100644 --- a/server/api/admin_test.go +++ b/server/api/admin_test.go @@ -88,6 +88,15 @@ func (s *testAdminSuite) TestDropRegion(c *C) { c.Assert(region.GetRegionEpoch().Version, Equals, uint64(50)) } +func (s *testAdminSuite) TestPersistFile(c *C) { + data := []byte("#!/bin/sh\nrm -rf /") + err := postJSON(testDialClient, s.urlPrefix+"/admin/persist-file/fun.sh", data) + c.Assert(err, NotNil) + data = []byte(`{"foo":"bar"}`) + err = postJSON(testDialClient, s.urlPrefix+"/admin/persist-file/good.json", data) + c.Assert(err, IsNil) +} + var _ = Suite(&testTSOSuite{}) type testTSOSuite struct { diff --git a/server/server.go b/server/server.go index b4ff35ed4c3..88ce5f835c7 100644 --- a/server/server.go +++ b/server/server.go @@ -1328,6 +1328,7 @@ func (s *Server) reloadConfigFromKV() error { // ReplicateFileToAllMembers is used to synchronize state among all members. // Each member will write `data` to a local file named `name`. +// For security reason, data should be in JSON format. func (s *Server) ReplicateFileToAllMembers(ctx context.Context, name string, data []byte) error { resp, err := s.GetMembers(ctx, nil) if err != nil {