Merge pull request #239 from kaleido-io/ip4-unsafe-addr-fix

[subscriptions] Fix for Validating URLs

internal/events/eventstream.go

@@ -764,7 +764,8 @@ func (a *eventStream) performActionWithRetry(batchNumber uint64, events []*event
 func (a *eventStream) isAddressUnsafe(ip *net.IPAddr) bool {
 	ip4 := ip.IP.To4()
 	return !a.allowPrivateIPs &&
-		(ip4[0] == 0 ||
+		(len(ip4) < 1 ||
+			ip4[0] == 0 ||
 			ip4[0] >= 224 ||
 			ip4[0] == 127 ||
 			ip4[0] == 10 ||

internal/events/webhooks.go

@@ -54,16 +54,10 @@ func newWebhookAction(es *eventStream, spec *webhookActionInfo) (*webhookAction,
 func (w *webhookAction) attemptBatch(batchNumber, attempt uint64, events []*eventData) error {
 	// We perform DNS resolution before each attempt, to exclude private IP address ranges from the target
 	esID := w.es.spec.ID
-	u, _ := url.Parse(w.spec.URL)
-	addr, err := net.ResolveIPAddr("ip4", u.Hostname())
+	u, addr, err := w.validateURL()
 	if err != nil {
 		return err
 	}
-	if w.es.isAddressUnsafe(addr) {
-		err := errors.Errorf(errors.EventStreamsWebhookProhibitedAddress, u.Hostname())
-		log.Errorf(err.Error())
-		return err
-	}
 	// Set the timeout
 	var transport = &http.Transport{
 		Proxy: http.ProxyFromEnvironment,
@@ -114,3 +108,20 @@ func (w *webhookAction) attemptBatch(batchNumber, attempt uint64, events []*even
 	}
 	return err
 }
+
+func (w *webhookAction) validateURL() (*url.URL, *net.IPAddr, error) {
+	u, err := url.Parse(w.spec.URL)
+	if err != nil {
+		return nil, nil, err
+	}
+	addr, err := net.ResolveIPAddr("ip4", u.Hostname())
+	if err != nil {
+		return nil, nil, err
+	}
+	if w.es.isAddressUnsafe(addr) {
+		err := errors.Errorf(errors.EventStreamsWebhookProhibitedAddress, u.Hostname())
+		log.Errorf(err.Error())
+		return nil, nil, err
+	}
+	return u, addr, nil
+}

internal/events/webhooks_test.go

@@ -0,0 +1,24 @@
+package events
+
+import (
+  "github.com/stretchr/testify/assert"
+  "testing"
+)
+
+func TestValidateURL(t *testing.T) {
+  w := &webhookAction{
+    es: &eventStream{
+      allowPrivateIPs: false,
+    },
+    spec: &webhookActionInfo{
+      URL: "badurl",
+    },
+  }
+
+  _, _, err := w.validateURL()
+  assert.Error(t, err)
+
+  w.spec.URL = "https://google.com"
+  _, _, err = w.validateURL()
+  assert.NoError(t, err)
+}