Skip to content

Commit 2794e12

Browse files
author
llhuii
committed
Fix CWE-117 reported by CodeQL checker
Signed-off-by: llhuii <[email protected]>
1 parent 5e329de commit 2794e12

File tree

2 files changed

+65
-9
lines changed

2 files changed

+65
-9
lines changed

pkg/globalmanager/messagelayer/ws/server.go

+25-5
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ limitations under the License.
1717
package ws
1818

1919
import (
20+
"fmt"
2021
"net/http"
22+
"strings"
2123

24+
"k8s.io/apimachinery/pkg/util/validation"
2225
"k8s.io/klog/v2"
2326

2427
"github.com/gorilla/websocket"
@@ -56,16 +59,35 @@ func (srv *Server) upgrade(w http.ResponseWriter, r *http.Request) *websocket.Co
5659
return conn
5760
}
5861

62+
func validateNodeName(rawNodeName string) (nodeName string, err error) {
63+
// Node name follows DNS Subdomain constraint
64+
// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names
65+
errs := validation.IsDNS1123Subdomain(rawNodeName)
66+
nodeName = strings.ReplaceAll(rawNodeName, "\n", "")
67+
nodeName = strings.ReplaceAll(nodeName, "\r", "")
68+
if len(errs) > 0 {
69+
err = fmt.Errorf("invalid node name %s: %s", nodeName, strings.Join(errs, ","))
70+
nodeName = ""
71+
}
72+
return
73+
}
74+
5975
func (srv *Server) ServeHTTP(w http.ResponseWriter, req *http.Request) {
60-
nodeName := req.Header.Get("Node-Name")
76+
rawNodeName := req.Header.Get("Node-Name")
77+
78+
nodeName, err := validateNodeName(rawNodeName)
79+
if err != nil {
80+
klog.Warningf("closing the connection, due to: %v", err)
81+
return
82+
}
6183
wsConn := srv.upgrade(w, req)
6284
if wsConn == nil {
6385
klog.Errorf("failed to upgrade to websocket for node %s", nodeName)
6486
return
6587
}
6688

6789
// serve connection
68-
nodeClient := &nodeClient{conn: wsConn, req: req}
90+
nodeClient := &nodeClient{conn: wsConn, req: req, nodeName: nodeName}
6991
go nodeClient.Serve()
7092
}
7193

@@ -104,10 +126,8 @@ func (nc *nodeClient) writeOneMsg(msg model.Message) error {
104126
}
105127

106128
func (nc *nodeClient) Serve() {
107-
nodeName := nc.req.Header.Get("Node-Name")
108-
nc.nodeName = nodeName
129+
nodeName := nc.nodeName
109130
klog.Infof("established connection for node %s", nodeName)
110-
// nc.conn.SetCloseHandler
111131
closeCh := make(chan struct{}, 2)
112132
AddNode(nodeName, nc.readOneMsg, nc.writeOneMsg, closeCh)
113133
<-closeCh

pkg/localcontroller/gmclient/websocket.go

+40-4
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@ import (
2121
"fmt"
2222
"net/http"
2323
"net/url"
24+
"strings"
2425
"time"
2526

2627
"github.com/gorilla/websocket"
28+
"k8s.io/apimachinery/pkg/util/validation"
2729
"k8s.io/klog/v2"
2830

2931
"github.com/kubeedge/sedna/cmd/sedna-lc/app/options"
@@ -76,6 +78,34 @@ func (c *wsClient) Subscribe(m MessageResourceHandler) error {
7678
return nil
7779
}
7880

81+
func sanitizeHeaderField(rawStr string) string {
82+
return strings.ReplaceAll(strings.ReplaceAll(rawStr, "\n", ""), "\r", "")
83+
}
84+
85+
func sanitizeHeader(h MessageHeader) MessageHeader {
86+
h.ResourceName = sanitizeHeaderField(h.ResourceName)
87+
h.Namespace = sanitizeHeaderField(h.Namespace)
88+
h.ResourceKind = sanitizeHeaderField(h.ResourceKind)
89+
h.Operation = sanitizeHeaderField(h.Operation)
90+
return h
91+
}
92+
93+
func validateMessage(msg *Message) (err error) {
94+
// ResourceName/Namespace follow 'RFC 1123 Label Names' constraint
95+
// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/
96+
errs := validation.IsDNS1123Label(msg.Header.ResourceName)
97+
if len(errs) > 0 {
98+
err = fmt.Errorf("invalid resource name: %s", strings.Join(errs, ","))
99+
return
100+
}
101+
errs = validation.IsDNS1123Label(msg.Header.Namespace)
102+
if len(errs) > 0 {
103+
err = fmt.Errorf("invalid namespace: %s", strings.Join(errs, ","))
104+
return
105+
}
106+
return
107+
}
108+
79109
// handleReceivedMessage handles received message
80110
func (c *wsClient) handleReceivedMessage(stop chan struct{}) {
81111
defer func() {
@@ -85,16 +115,22 @@ func (c *wsClient) handleReceivedMessage(stop chan struct{}) {
85115
ws := c.WSConnection.WSConn
86116

87117
for {
88-
message := Message{}
118+
var message Message
89119
if err := ws.ReadJSON(&message); err != nil {
90-
klog.Errorf("client received message from global manager(address: %s) failed, error: %v",
120+
klog.Errorf("client failed to read message from gm(%s): %v",
91121
c.Options.GMAddr, err)
92122
return
93123
}
124+
if err := validateMessage(&message); err != nil {
125+
klog.Warningf("failed to validate message from gm(%s): %v",
126+
message.Header, c.Options.GMAddr, err)
127+
continue
128+
}
129+
message.Header = sanitizeHeader(message.Header)
94130

95-
klog.V(2).Infof("client received message header: %+v from global manager(address: %s)",
131+
klog.V(2).Infof("client received message header: %+v from gm(%s)",
96132
message.Header, c.Options.GMAddr)
97-
klog.V(4).Infof("client received message content: %s from global manager(address: %s)",
133+
klog.V(4).Infof("client received message content: %s from gm(%s)",
98134
message.Content, c.Options.GMAddr)
99135

100136
m := c.SubscribeMessageMap[message.Header.ResourceKind]

0 commit comments

Comments
 (0)