Skip to content

Commit

Permalink
use status.Error instead of status.Errorf (#397)
Browse files Browse the repository at this point in the history
Use `status.Error` instead of `status.Errorf` when the format string is
non-constant and not actually a format string. In the case of the
validator middleware, the error being supplied as a format string could
potentially contain data supplied by an attacker allowing for format
string injection. This doesn't appear to be an actual problem due to
`fmt` being safe in this regards, but it certainly isn't good practice
to provide a format string that an attacker can control.

Fixes #396
  • Loading branch information
petermattis committed Feb 11, 2021
1 parent fab13c2 commit 912313c
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 7 deletions.
10 changes: 6 additions & 4 deletions retry/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,9 @@ func (s *serverStreamingRetryingStream) receiveMsgAndIndicateRetry(m interface{}
return isRetriable(err, s.callOpts), err
}

func (s *serverStreamingRetryingStream) reestablishStreamAndResendBuffer(callCtx context.Context) (grpc.ClientStream, error) {
func (s *serverStreamingRetryingStream) reestablishStreamAndResendBuffer(
callCtx context.Context,
) (grpc.ClientStream, error) {
s.mu.RLock()
bufferedSends := s.bufferedSends
s.mu.RUnlock()
Expand Down Expand Up @@ -310,11 +312,11 @@ func perCallContext(parentCtx context.Context, callOpts *options, attempt uint)
func contextErrToGrpcErr(err error) error {
switch err {
case context.DeadlineExceeded:
return status.Errorf(codes.DeadlineExceeded, err.Error())
return status.Error(codes.DeadlineExceeded, err.Error())
case context.Canceled:
return status.Errorf(codes.Canceled, err.Error())
return status.Error(codes.Canceled, err.Error())
default:
return status.Errorf(codes.Unknown, err.Error())
return status.Error(codes.Unknown, err.Error())
}
}

Expand Down
6 changes: 3 additions & 3 deletions validator/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func UnaryServerInterceptor() grpc.UnaryServerInterceptor {
return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
if v, ok := req.(validator); ok {
if err := v.Validate(); err != nil {
return nil, status.Errorf(codes.InvalidArgument, err.Error())
return nil, status.Error(codes.InvalidArgument, err.Error())
}
}
return handler(ctx, req)
Expand All @@ -36,7 +36,7 @@ func UnaryClientInterceptor() grpc.UnaryClientInterceptor {
return func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
if v, ok := req.(validator); ok {
if err := v.Validate(); err != nil {
return status.Errorf(codes.InvalidArgument, err.Error())
return status.Error(codes.InvalidArgument, err.Error())
}
}
return invoker(ctx, method, req, reply, cc, opts...)
Expand Down Expand Up @@ -66,7 +66,7 @@ func (s *recvWrapper) RecvMsg(m interface{}) error {
}
if v, ok := m.(validator); ok {
if err := v.Validate(); err != nil {
return status.Errorf(codes.InvalidArgument, err.Error())
return status.Error(codes.InvalidArgument, err.Error())
}
}
return nil
Expand Down

0 comments on commit 912313c

Please sign in to comment.