Skip to content

Commit

Permalink
Audit code for null ptr exceptions. (Velocidex#598)
Browse files Browse the repository at this point in the history
  • Loading branch information
scudette authored Aug 29, 2020
1 parent 968366c commit d983110
Show file tree
Hide file tree
Showing 54 changed files with 436 additions and 145 deletions.
2 changes: 1 addition & 1 deletion acls/acls.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func CheckAccess(
permissions ...ACL_PERMISSION) (bool, error) {

// Internal calls from the server are allowed to do anything.
if principal == config_obj.Client.PinnedServerName {
if config_obj.Client != nil && principal == config_obj.Client.PinnedServerName {
return true, nil
}

Expand Down
15 changes: 14 additions & 1 deletion api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func (self *ApiServer) GetReport(
func (self *ApiServer) CollectArtifact(
ctx context.Context,
in *flows_proto.ArtifactCollectorArgs) (*flows_proto.ArtifactCollectorResponse, error) {

result := &flows_proto.ArtifactCollectorResponse{Request: in}
creator := GetGRPCUserInfo(self.config, ctx).Name

Expand Down Expand Up @@ -1028,6 +1029,13 @@ func startAPIServer(
wg *sync.WaitGroup,
config_obj *config_proto.Config,
server_obj *server.Server) error {

if config_obj.API == nil ||
config_obj.Client == nil ||
config_obj.Frontend == nil {
return errors.New("API server not configured")
}

bind_addr := config_obj.API.BindAddress
switch config_obj.API.BindScheme {
case "tcp":
Expand Down Expand Up @@ -1099,7 +1107,11 @@ func startAPIServer(
func StartMonitoringService(
ctx context.Context,
wg *sync.WaitGroup,
config_obj *config_proto.Config) {
config_obj *config_proto.Config) error {

if config_obj.Monitoring == nil {
return nil
}

logger := logging.GetLogger(config_obj, &logging.FrontendComponent)
bind_addr := fmt.Sprintf("%s:%d",
Expand Down Expand Up @@ -1143,4 +1155,5 @@ func StartMonitoringService(
}()

logger.Info("Launched Prometheus monitoring server on %v ", bind_addr)
return nil
}
4 changes: 4 additions & 0 deletions api/artifacts.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ func searchArtifact(
number_of_results uint64) (
*artifacts_proto.ArtifactDescriptors, error) {

if config_obj.GUI == nil {
return nil, errors.New("GUI not configured")
}

name_filter_regexp := config_obj.GUI.ArtifactSearchFilter
if name_filter_regexp == "" {
name_filter_regexp = "."
Expand Down
5 changes: 4 additions & 1 deletion api/assets.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ import (
)

func install_static_assets(config_obj *config_proto.Config, mux *http.ServeMux) {
base := config_obj.GUI.BasePath
base := ""
if config_obj.GUI != nil {
base = config_obj.GUI.BasePath
}
dir := base + "/static/"
mux.Handle(dir, http.StripPrefix(base, http.FileServer(assets.HTTP)))
mux.Handle("/favicon.png",
Expand Down
9 changes: 8 additions & 1 deletion api/assets_filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
package api

import (
"errors"
"html/template"
"net/http"
"time"
Expand All @@ -35,7 +36,11 @@ import (
"www.velocidex.com/golang/velociraptor/logging"
)

func install_static_assets(config_obj *config_proto.Config, mux *http.ServeMux) {
func install_static_assets(config_obj *config_proto.Config, mux *http.ServeMux) error {
if config_obj.GUI == nil {
return errors.New("GUI not configured")
}

base := config_obj.GUI.BasePath
logging.GetLogger(config_obj, &logging.FrontendComponent).
Info("GUI will serve files from directory gui/static")
Expand All @@ -50,6 +55,8 @@ func install_static_assets(config_obj *config_proto.Config, mux *http.ServeMux)
mux.Handle(base+"/favicon.ico",
http.RedirectHandler(base+"/static/images/favicon.ico",
http.StatusMovedPermanently))

return nil
}

func GetTemplateHandler(config_obj *config_proto.Config,
Expand Down
23 changes: 11 additions & 12 deletions api/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ import (
"www.velocidex.com/golang/velociraptor/logging"
)

var (
contextKeyUser = "USER"
)

// GetGRPCUserInfo: Extracts user information from GRPC context.
func GetGRPCUserInfo(
config_obj *config_proto.Config,
Expand All @@ -42,7 +38,7 @@ func GetGRPCUserInfo(
peer, ok := peer.FromContext(ctx)
if ok {
tlsInfo, ok := peer.AuthInfo.(credentials.TLSInfo)
if ok {
if ok && config_obj.API != nil {
v := tlsInfo.State.PeerCertificates[0].Subject.CommonName

// Calls from the gRPC gateway are allowed to
Expand Down Expand Up @@ -82,16 +78,19 @@ func GetGRPCUserInfo(

func NewDefaultUserObject(config_obj *config_proto.Config) *api_proto.ApiGrrUser {
result := &api_proto.ApiGrrUser{
InterfaceTraits: &api_proto.ApiGrrUserInterfaceTraits{
AuthUsingGoogle: config_obj.GUI.GoogleOauthClientId != "",
Links: []*api_proto.UILink{},
},
UserType: api_proto.ApiGrrUser_USER_TYPE_ADMIN,
}

for _, link := range config_obj.GUI.Links {
result.InterfaceTraits.Links = append(result.InterfaceTraits.Links,
&api_proto.UILink{Text: link.Text, Url: link.Url})
if config_obj.GUI != nil {
result.InterfaceTraits = &api_proto.ApiGrrUserInterfaceTraits{
AuthUsingGoogle: config_obj.GUI.GoogleOauthClientId != "",
Links: []*api_proto.UILink{},
}

for _, link := range config_obj.GUI.Links {
result.InterfaceTraits.Links = append(result.InterfaceTraits.Links,
&api_proto.UILink{Text: link.Text, Url: link.Url})
}
}

return result
Expand Down
4 changes: 3 additions & 1 deletion api/authenticators/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ type Authenticator interface {
}

func NewAuthenticator(config_obj *config_proto.Config) (Authenticator, error) {
if config_obj.GUI == nil || config_obj.GUI.Authenticator == nil {
if config_obj.GUI == nil ||
config_obj.GUI.Authenticator == nil ||
config_obj.Frontend == nil {
return nil, errors.New("GUI not configured")
}

Expand Down
57 changes: 48 additions & 9 deletions api/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package api
import (
"context"
"crypto/tls"
"errors"
"fmt"
"net/http"
"sync"
Expand Down Expand Up @@ -44,10 +45,14 @@ type Builder struct {

func (self *Builder) StartServer(ctx context.Context, wg *sync.WaitGroup) error {
// Always start the prometheus monitoring service
StartMonitoringService(ctx, wg, self.config_obj)
err := StartMonitoringService(ctx, wg, self.config_obj)
if err != nil {
return err
}

// Start in autocert mode, only put the GUI behind autocert if the GUI port is 443.
if self.AutocertCertCache != "" && self.config_obj.GUI.BindPort == 443 {
if self.AutocertCertCache != "" && self.config_obj.GUI != nil &&
self.config_obj.GUI.BindPort == 443 {
return self.WithAutocertGUI(ctx, wg)
}

Expand Down Expand Up @@ -96,15 +101,18 @@ func (self *Builder) withAutoCertFrontendSelfSignedGUI(
wg *sync.WaitGroup,
config_obj *config_proto.Config,
server_obj *server.Server) error {
logger := logging.Manager.GetLogger(config_obj, &logging.GUIComponent)

logger.Info("Autocert is enabled but GUI port is not 443, starting Frontend with autocert and GUI with self signed.")
if self.config_obj.Frontend == nil || self.config_obj.GUI == nil {
return errors.New("Frontend not configured")
}

if config_obj.Frontend.ServerServices.GuiServer {
logger := logging.GetLogger(config_obj, &logging.GUIComponent)
logger.Info("Autocert is enabled but GUI port is not 443, starting Frontend with autocert and GUI with self signed.")

if config_obj.Frontend.ServerServices.GuiServer && config_obj.GUI != nil {
mux := http.NewServeMux()

router, err := PrepareGUIMux(config_obj, mux)
router, err := PrepareGUIMux(ctx, config_obj, mux)
if err != nil {
return err
}
Expand Down Expand Up @@ -137,10 +145,14 @@ func (self *Builder) WithAutocertGUI(
ctx context.Context,
wg *sync.WaitGroup) error {

if self.config_obj.Frontend == nil || self.config_obj.GUI == nil {
return errors.New("Frontend not configured")
}

mux := http.NewServeMux()

server.PrepareFrontendMux(self.config_obj, self.server_obj, mux)
router, err := PrepareGUIMux(self.config_obj, mux)
router, err := PrepareGUIMux(ctx, self.config_obj, mux)
if err != nil {
return err
}
Expand All @@ -159,8 +171,12 @@ func startSharedSelfSignedFrontend(
server_obj *server.Server) error {
mux := http.NewServeMux()

if config_obj.Frontend == nil || config_obj.GUI == nil {
return errors.New("Frontend not configured")
}

server.PrepareFrontendMux(config_obj, server_obj, mux)
router, err := PrepareGUIMux(config_obj, mux)
router, err := PrepareGUIMux(ctx, config_obj, mux)
if err != nil {
return err
}
Expand All @@ -183,11 +199,15 @@ func startSelfSignedFrontend(
config_obj *config_proto.Config,
server_obj *server.Server) error {

if config_obj.Frontend == nil {
return errors.New("Frontend not configured")
}

// Launch a new server for the GUI.
if config_obj.Frontend.ServerServices.GuiServer {
mux := http.NewServeMux()

router, err := PrepareGUIMux(config_obj, mux)
router, err := PrepareGUIMux(ctx, config_obj, mux)
if err != nil {
return err
}
Expand Down Expand Up @@ -227,6 +247,10 @@ func StartFrontendHttps(
server_obj *server.Server,
router http.Handler) error {

if config_obj.Frontend == nil {
return errors.New("Frontend server not configured")
}

cert, err := tls.X509KeyPair(
[]byte(config_obj.Frontend.Certificate),
[]byte(config_obj.Frontend.PrivateKey))
Expand Down Expand Up @@ -315,6 +339,9 @@ func StartFrontendPlainHttp(
config_obj *config_proto.Config,
server_obj *server.Server,
router http.Handler) error {
if config_obj.Frontend == nil {
return errors.New("Frontend server not configured")
}

listenAddr := fmt.Sprintf(
"%s:%d",
Expand Down Expand Up @@ -383,6 +410,10 @@ func StartFrontendWithAutocert(
server_obj *server.Server,
mux http.Handler) error {

if config_obj.Frontend == nil {
return errors.New("Frontend server not configured")
}

logger := logging.Manager.GetLogger(config_obj, &logging.GUIComponent)

cache_dir := config_obj.AutocertCertCache
Expand Down Expand Up @@ -458,6 +489,11 @@ func StartHTTPGUI(
ctx context.Context,
wg *sync.WaitGroup,
config_obj *config_proto.Config, mux http.Handler) error {

if config_obj.GUI == nil {
return errors.New("GUI server not configured")
}

logger := logging.Manager.GetLogger(config_obj, &logging.GUIComponent)

listenAddr := fmt.Sprintf("%s:%d",
Expand Down Expand Up @@ -514,6 +550,9 @@ func StartSelfSignedGUI(
wg *sync.WaitGroup,
config_obj *config_proto.Config, mux http.Handler) error {
logger := logging.Manager.GetLogger(config_obj, &logging.GUIComponent)
if config_obj.GUI == nil {
return errors.New("GUI server not configured")
}

cert, err := tls.X509KeyPair(
[]byte(config_obj.Frontend.Certificate),
Expand Down
4 changes: 4 additions & 0 deletions api/clients.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ func GetApiClient(
client_id string, detailed bool) (
*api_proto.ApiClient, error) {

if config_obj.GUI == nil {
return nil, errors.New("GUI not configured")
}

result := &api_proto.ApiClient{
ClientId: client_id,
}
Expand Down
24 changes: 20 additions & 4 deletions api/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"www.velocidex.com/golang/velociraptor/api/authenticators"
api_proto "www.velocidex.com/golang/velociraptor/api/proto"
config_proto "www.velocidex.com/golang/velociraptor/config/proto"
"www.velocidex.com/golang/velociraptor/constants"
"www.velocidex.com/golang/velociraptor/crypto"
file_store "www.velocidex.com/golang/velociraptor/file_store"
"www.velocidex.com/golang/velociraptor/file_store/api"
Expand All @@ -42,7 +43,11 @@ import (

// A Mux for the reverse proxy feature.
func AddProxyMux(config_obj *config_proto.Config, mux *http.ServeMux) error {
logger := logging.Manager.GetLogger(config_obj, &logging.GUIComponent)
if config_obj.GUI == nil {
return errors.New("GUI not configured")
}

logger := logging.GetLogger(config_obj, &logging.GUIComponent)

for _, reverse_proxy_config := range config_obj.GUI.ReverseProxy {
target, err := url.Parse(reverse_proxy_config.Url)
Expand Down Expand Up @@ -94,8 +99,13 @@ func AddProxyMux(config_obj *config_proto.Config, mux *http.ServeMux) error {
}

// Prepares a mux for the GUI by adding handlers required by the GUI.
func PrepareGUIMux(config_obj *config_proto.Config, mux *http.ServeMux) (http.Handler, error) {
ctx := context.Background()
func PrepareGUIMux(
ctx context.Context,
config_obj *config_proto.Config, mux *http.ServeMux) (http.Handler, error) {
if config_obj.GUI == nil {
return nil, errors.New("GUI not configured")
}

h, err := GetAPIHandler(ctx, config_obj)
if err != nil {
return nil, err
Expand Down Expand Up @@ -193,6 +203,12 @@ func GetAPIHandler(
ctx context.Context,
config_obj *config_proto.Config) (http.Handler, error) {

if config_obj.Client == nil ||
config_obj.GUI == nil ||
config_obj.API == nil {
return nil, errors.New("Client not configured")
}

// We need to tell when someone uses HEAD method on our grpc
// proxy so we need to pass this information from the request
// to the gRPC server using the gRPC metadata.
Expand All @@ -203,7 +219,7 @@ func GetAPIHandler(
"METHOD": req.Method,
}
username, ok := req.Context().Value(
contextKeyUser).(string)
constants.GRPC_USER_CONTEXT).(string)
if ok {
md["USER"] = username
}
Expand Down
2 changes: 1 addition & 1 deletion api/reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func getArgDescriptors(arg_type string, type_map *vfilter.TypeMap,
scope *vfilter.Scope) []*api_proto.ArgDescriptor {
args := []*api_proto.ArgDescriptor{}
arg_desc, pres := type_map.Get(scope, arg_type)
if pres {
if pres && arg_desc != nil && arg_desc.Fields != nil {
for _, k := range arg_desc.Fields.Keys() {
v_any, _ := arg_desc.Fields.Get(k)
v, ok := v_any.(*vfilter.TypeReference)
Expand Down
Loading

0 comments on commit d983110

Please sign in to comment.