Skip to content

Commit 8e1f561

Browse files
nwidgernwidger
authored
Add more error checking (#26)
Mostly checking previously ignored `error` values and a `nil` returned by `net.ParseIP`. Only tricky bit is not parsing IP address string parameters unless they are non-zero length in certain cases. This is necessary since an empty string is used in certain situations to mean "do a wildcard bind" rather than "bind to this address".
1 parent 1f423b4 commit 8e1f561

File tree

5 files changed

+83
-26
lines changed

5 files changed

+83
-26
lines changed

‎api/server.go‎

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,14 @@ func NewServer(store *store.Store, authorization, rootPath string) (server *Serv
8181
var b []byte
8282
if strings.Contains(r.Header.Get("Accept"), "application/json") {
8383
w.Header().Set("Content-Type", "applications/json")
84-
b, _ = json.Marshal(m)
84+
b, err = json.Marshal(m)
8585
} else {
8686
w.Header().Set("Content-Type", "text/yaml")
87-
b, _ = yaml.Marshal(m)
87+
b, err = yaml.Marshal(m)
88+
}
89+
if err != nil {
90+
http.Error(w, "error marshalling manifest: "+err.Error(), http.StatusInternalServerError)
91+
return
8892
}
8993
w.WriteHeader(http.StatusOK)
9094
w.Write(b)
@@ -100,10 +104,14 @@ func NewServer(store *store.Store, authorization, rootPath string) (server *Serv
100104
var b []byte
101105
if strings.Contains(r.Header.Get("Accept"), "application/json") {
102106
w.Header().Set("Content-Type", "applications/json")
103-
b, _ = json.Marshal(store.GetAll())
107+
b, err = json.Marshal(store.GetAll())
104108
} else {
105109
w.Header().Set("Content-Type", "text/yaml")
106-
b, _ = yaml.Marshal(store.GetAll())
110+
b, err = yaml.Marshal(store.GetAll())
111+
}
112+
if err != nil {
113+
http.Error(w, "error marshalling manifests: "+err.Error(), http.StatusInternalServerError)
114+
return
107115
}
108116
w.WriteHeader(http.StatusOK)
109117
w.Write(b)
@@ -121,17 +129,21 @@ func NewServer(store *store.Store, authorization, rootPath string) (server *Serv
121129
if r.Header.Get("Content-Type") == "application/json" {
122130
m, err = manifest.ManifestFromJson(buf, rootPath)
123131
if err != nil {
124-
http.Error(w, err.Error(), http.StatusBadRequest)
132+
http.Error(w, "error loading manifest from json: "+err.Error(), http.StatusBadRequest)
125133
return
126134
}
127135
} else {
128136
m, err = manifest.ManifestFromYaml(buf, rootPath)
129137
if err != nil {
130-
http.Error(w, err.Error(), http.StatusBadRequest)
138+
http.Error(w, "error loading manifest from yaml: "+err.Error(), http.StatusBadRequest)
131139
return
132140
}
133141
}
134-
_ = store.PutManifest(m)
142+
err = store.PutManifest(m)
143+
if err != nil {
144+
http.Error(w, "error storing manifest: "+err.Error(), http.StatusBadRequest)
145+
return
146+
}
135147
w.WriteHeader(http.StatusCreated)
136148
}).Methods("PUT")
137149

@@ -150,16 +162,20 @@ func NewServer(store *store.Store, authorization, rootPath string) (server *Serv
150162

151163
// GET|POST /api/self/suspend-boot
152164
r.HandleFunc("/api/self/suspend-boot", func(w http.ResponseWriter, r *http.Request) {
153-
var ip net.IP
165+
var ipStr string
154166
if queryFirst(r, "spoof") != "" {
155167
if authorization != r.Header.Get("Authorization") {
156168
http.Error(w, "Forbidden", http.StatusForbidden)
157169
return
158170
}
159-
ip = net.ParseIP(queryFirst(r, "spoof"))
171+
ipStr = queryFirst(r, "spoof")
160172
} else {
161-
host, _, _ := net.SplitHostPort(r.RemoteAddr)
162-
ip = net.ParseIP(host)
173+
ipStr, _, _ = net.SplitHostPort(r.RemoteAddr)
174+
}
175+
ip := net.ParseIP(ipStr)
176+
if ip == nil {
177+
http.Error(w, "could not determine host's ip: invalid ip: "+ipStr, http.StatusInternalServerError)
178+
return
163179
}
164180

165181
m := server.store.FindByIP(ip)
@@ -174,16 +190,20 @@ func NewServer(store *store.Store, authorization, rootPath string) (server *Serv
174190

175191
// GET|POST /api/self/unsuspend-boot
176192
r.HandleFunc("/api/self/unsuspend-boot", func(w http.ResponseWriter, r *http.Request) {
177-
var ip net.IP
193+
var ipStr string
178194
if queryFirst(r, "spoof") != "" {
179195
if authorization != r.Header.Get("Authorization") {
180196
http.Error(w, "Forbidden", http.StatusForbidden)
181197
return
182198
}
183-
ip = net.ParseIP(queryFirst(r, "spoof"))
199+
ipStr = queryFirst(r, "spoof")
184200
} else {
185-
host, _, _ := net.SplitHostPort(r.RemoteAddr)
186-
ip = net.ParseIP(host)
201+
ipStr, _, _ = net.SplitHostPort(r.RemoteAddr)
202+
}
203+
ip := net.ParseIP(ipStr)
204+
if ip == nil {
205+
http.Error(w, "could not determine host's ip: invalid ip: "+ipStr, http.StatusInternalServerError)
206+
return
187207
}
188208

189209
m := server.store.FindByIP(ip)
@@ -198,16 +218,20 @@ func NewServer(store *store.Store, authorization, rootPath string) (server *Serv
198218

199219
// GET /api/self/manifest
200220
r.HandleFunc("/api/self/manifest", func(w http.ResponseWriter, r *http.Request) {
201-
var ip net.IP
221+
var ipStr string
202222
if queryFirst(r, "spoof") != "" {
203223
if authorization != r.Header.Get("Authorization") {
204224
http.Error(w, "Forbidden", http.StatusForbidden)
205225
return
206226
}
207-
ip = net.ParseIP(queryFirst(r, "spoof"))
227+
ipStr = queryFirst(r, "spoof")
208228
} else {
209-
host, _, _ := net.SplitHostPort(r.RemoteAddr)
210-
ip = net.ParseIP(host)
229+
ipStr, _, _ = net.SplitHostPort(r.RemoteAddr)
230+
}
231+
ip := net.ParseIP(ipStr)
232+
if ip == nil {
233+
http.Error(w, "could not determine host's ip: invalid ip: "+ipStr, http.StatusInternalServerError)
234+
return
211235
}
212236

213237
m := server.store.FindByIP(ip)
@@ -218,10 +242,14 @@ func NewServer(store *store.Store, authorization, rootPath string) (server *Serv
218242
var b []byte
219243
if strings.Contains(r.Header.Get("Accept"), "application/json") {
220244
w.Header().Set("Content-Type", "applications/json")
221-
b, _ = json.Marshal(store.GetAll())
245+
b, err = json.Marshal(store.GetAll())
222246
} else {
223247
w.Header().Set("Content-Type", "text/yaml")
224-
b, _ = yaml.Marshal(store.GetAll())
248+
b, err = yaml.Marshal(store.GetAll())
249+
}
250+
if err != nil {
251+
http.Error(w, "error marshalling manifest: "+err.Error(), http.StatusInternalServerError)
252+
return
225253
}
226254
w.WriteHeader(http.StatusOK)
227255
w.Write(b)

‎cmd/arpinject.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ var arpInjectCmd = &cobra.Command{
2828
Run: func(cmd *cobra.Command, args []string) {
2929
config.InitZeroLog()
3030
parsedIp := net.ParseIP(ip)
31+
if parsedIp == nil {
32+
log.Fatal().Msgf("invalid ip: %s", ip)
33+
}
3134
parsedMac, err := net.ParseMAC(mac)
3235
if err != nil {
3336
log.Error().

‎cmd/server.go‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,10 @@ var serverCmd = &cobra.Command{
7777
})
7878
if viper.GetString("manifestPath") != "" {
7979
log.Info().Str("path", viper.GetString("manifestPath")).Msg("Loading manifests")
80-
_ = store.LoadFromDirectory(viper.GetString("manifestPath"), viper.GetString("rootPath"))
80+
err := store.LoadFromDirectory(viper.GetString("manifestPath"), viper.GetString("rootPath"))
81+
if err != nil {
82+
log.Fatal().Err(err).Msg("Failed to load manifests")
83+
}
8184
}
8285
store.GlobalHints.HttpPort = viper.GetInt("http.port")
8386
store.GlobalHints.ApiPort = viper.GetInt("api.port")
@@ -89,13 +92,23 @@ var serverCmd = &cobra.Command{
8992
}
9093
go dhcpServer.Serve()
9194

95+
var addressIP net.IP
96+
addressIPStr := viper.GetString("address")
97+
// only parse addressIPStr if non-zero length
98+
if addressIPStr != "" {
99+
addressIP = net.ParseIP(addressIPStr)
100+
if addressIP == nil {
101+
log.Fatal().Msgf("Invalid address: %s", addressIPStr)
102+
}
103+
}
104+
92105
// TFTP
93106
tftpServer, err := tftpd.NewServer(store, viper.GetString("rootPath"))
94107
if err != nil {
95108
log.Fatal().Err(err).Msg("Failed to create TFTP server")
96109
}
97110
connTftp, err := net.ListenUDP("udp", &net.UDPAddr{
98-
IP: net.ParseIP(viper.GetString("address")),
111+
IP: addressIP,
99112
Port: 69, // TFTP
100113
})
101114
if err != nil {
@@ -109,7 +122,7 @@ var serverCmd = &cobra.Command{
109122
log.Fatal().Err(err).Msg("Failed to create HTTP server")
110123
}
111124
connHttp, err := net.ListenTCP("tcp", &net.TCPAddr{
112-
IP: net.ParseIP(viper.GetString("address")),
125+
IP: addressIP,
113126
Port: viper.GetInt("http.port"), // HTTP
114127
})
115128
if err != nil {
@@ -124,7 +137,7 @@ var serverCmd = &cobra.Command{
124137
log.Fatal().Err(err).Msg("Failed to create HTTP API server")
125138
}
126139
connApi, err := net.ListenTCP("tcp", &net.TCPAddr{
127-
IP: net.ParseIP(viper.GetString("address")),
140+
IP: addressIP,
128141
Port: viper.GetInt("api.port"), // HTTP
129142
})
130143
if err != nil {

‎dhcpd/server.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package dhcpd
44

55
import (
6+
"fmt"
67
"net"
78

89
"github.com/DSpeichert/netbootd/store"
@@ -22,9 +23,17 @@ type Server struct {
2223
}
2324

2425
func NewServer(addr, ifname string, store *store.Store) (server *Server, err error) {
26+
var ip net.IP
27+
// only parse addr if non-zero length
28+
if addr != "" {
29+
ip = net.ParseIP(addr)
30+
if ip == nil {
31+
return nil, fmt.Errorf("invalid ip: %s", addr)
32+
}
33+
}
2534
server = &Server{
2635
address: &net.UDPAddr{
27-
IP: net.ParseIP(addr),
36+
IP: ip,
2837
Port: 67,
2938
Zone: ifname,
3039
},

‎httpd/handler.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
7373
spoofIPs, ok := r.URL.Query()["spoof"]
7474
if ok && len(spoofIPs[0]) > 0 {
7575
manifestRaddr = net.ParseIP(spoofIPs[0])
76+
if manifestRaddr == nil {
77+
http.Error(w, "unable to determine host address: invalid ip: "+spoofIPs[0], http.StatusBadRequest)
78+
return
79+
}
7680
}
7781

7882
manifest := h.server.store.FindByIP(manifestRaddr)

0 commit comments

Comments
 (0)