From 2794e12c845b88a9c8d1976643e7a920c341b3f7 Mon Sep 17 00:00:00 2001 From: llhuii Date: Wed, 15 Dec 2021 18:01:00 +0800 Subject: [PATCH] Fix CWE-117 reported by CodeQL checker Signed-off-by: llhuii --- pkg/globalmanager/messagelayer/ws/server.go | 30 +++++++++++--- pkg/localcontroller/gmclient/websocket.go | 44 +++++++++++++++++++-- 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/pkg/globalmanager/messagelayer/ws/server.go b/pkg/globalmanager/messagelayer/ws/server.go index ae54c8988..2fd2419ba 100644 --- a/pkg/globalmanager/messagelayer/ws/server.go +++ b/pkg/globalmanager/messagelayer/ws/server.go @@ -17,8 +17,11 @@ limitations under the License. package ws import ( + "fmt" "net/http" + "strings" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/klog/v2" "github.com/gorilla/websocket" @@ -56,8 +59,27 @@ func (srv *Server) upgrade(w http.ResponseWriter, r *http.Request) *websocket.Co return conn } +func validateNodeName(rawNodeName string) (nodeName string, err error) { + // Node name follows DNS Subdomain constraint + // https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names + errs := validation.IsDNS1123Subdomain(rawNodeName) + nodeName = strings.ReplaceAll(rawNodeName, "\n", "") + nodeName = strings.ReplaceAll(nodeName, "\r", "") + if len(errs) > 0 { + err = fmt.Errorf("invalid node name %s: %s", nodeName, strings.Join(errs, ",")) + nodeName = "" + } + return +} + func (srv *Server) ServeHTTP(w http.ResponseWriter, req *http.Request) { - nodeName := req.Header.Get("Node-Name") + rawNodeName := req.Header.Get("Node-Name") + + nodeName, err := validateNodeName(rawNodeName) + if err != nil { + klog.Warningf("closing the connection, due to: %v", err) + return + } wsConn := srv.upgrade(w, req) if wsConn == nil { klog.Errorf("failed to upgrade to websocket for node %s", nodeName) @@ -65,7 +87,7 @@ func (srv *Server) ServeHTTP(w http.ResponseWriter, req *http.Request) { } // serve connection - nodeClient := &nodeClient{conn: wsConn, req: req} + nodeClient := &nodeClient{conn: wsConn, req: req, nodeName: nodeName} go nodeClient.Serve() } @@ -104,10 +126,8 @@ func (nc *nodeClient) writeOneMsg(msg model.Message) error { } func (nc *nodeClient) Serve() { - nodeName := nc.req.Header.Get("Node-Name") - nc.nodeName = nodeName + nodeName := nc.nodeName klog.Infof("established connection for node %s", nodeName) - // nc.conn.SetCloseHandler closeCh := make(chan struct{}, 2) AddNode(nodeName, nc.readOneMsg, nc.writeOneMsg, closeCh) <-closeCh diff --git a/pkg/localcontroller/gmclient/websocket.go b/pkg/localcontroller/gmclient/websocket.go index fcfac2242..45a4fcf2c 100644 --- a/pkg/localcontroller/gmclient/websocket.go +++ b/pkg/localcontroller/gmclient/websocket.go @@ -21,9 +21,11 @@ import ( "fmt" "net/http" "net/url" + "strings" "time" "github.com/gorilla/websocket" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/klog/v2" "github.com/kubeedge/sedna/cmd/sedna-lc/app/options" @@ -76,6 +78,34 @@ func (c *wsClient) Subscribe(m MessageResourceHandler) error { return nil } +func sanitizeHeaderField(rawStr string) string { + return strings.ReplaceAll(strings.ReplaceAll(rawStr, "\n", ""), "\r", "") +} + +func sanitizeHeader(h MessageHeader) MessageHeader { + h.ResourceName = sanitizeHeaderField(h.ResourceName) + h.Namespace = sanitizeHeaderField(h.Namespace) + h.ResourceKind = sanitizeHeaderField(h.ResourceKind) + h.Operation = sanitizeHeaderField(h.Operation) + return h +} + +func validateMessage(msg *Message) (err error) { + // ResourceName/Namespace follow 'RFC 1123 Label Names' constraint + // https://kubernetes.io/docs/concepts/overview/working-with-objects/names/ + errs := validation.IsDNS1123Label(msg.Header.ResourceName) + if len(errs) > 0 { + err = fmt.Errorf("invalid resource name: %s", strings.Join(errs, ",")) + return + } + errs = validation.IsDNS1123Label(msg.Header.Namespace) + if len(errs) > 0 { + err = fmt.Errorf("invalid namespace: %s", strings.Join(errs, ",")) + return + } + return +} + // handleReceivedMessage handles received message func (c *wsClient) handleReceivedMessage(stop chan struct{}) { defer func() { @@ -85,16 +115,22 @@ func (c *wsClient) handleReceivedMessage(stop chan struct{}) { ws := c.WSConnection.WSConn for { - message := Message{} + var message Message if err := ws.ReadJSON(&message); err != nil { - klog.Errorf("client received message from global manager(address: %s) failed, error: %v", + klog.Errorf("client failed to read message from gm(%s): %v", c.Options.GMAddr, err) return } + if err := validateMessage(&message); err != nil { + klog.Warningf("failed to validate message from gm(%s): %v", + message.Header, c.Options.GMAddr, err) + continue + } + message.Header = sanitizeHeader(message.Header) - klog.V(2).Infof("client received message header: %+v from global manager(address: %s)", + klog.V(2).Infof("client received message header: %+v from gm(%s)", message.Header, c.Options.GMAddr) - klog.V(4).Infof("client received message content: %s from global manager(address: %s)", + klog.V(4).Infof("client received message content: %s from gm(%s)", message.Content, c.Options.GMAddr) m := c.SubscribeMessageMap[message.Header.ResourceKind]