Skip to content

Commit c7637a7

Browse files
Alvaro Muñozowen-mc
andauthored
Apply suggestions from code review
Co-authored-by: Owen Mansel-Chan <[email protected]>
1 parent a0cf8e7 commit c7637a7

File tree

2 files changed

+12
-10
lines changed
  • go/ql
    • lib/semmle/go/frameworks
    • test/library-tests/semmle/go/frameworks/Twirp/server

2 files changed

+12
-10
lines changed

go/ql/lib/semmle/go/frameworks/Twirp.qll

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ module Twirp {
2222

2323
/**
2424
* A *.twirp.go file generated by Twirp.
25+
*
2526
* This file contains all the types representing protobuf services and should have a companion *.pb.go file.
2627
*/
2728
class ServicesGeneratedFile extends File {
@@ -66,7 +67,7 @@ module Twirp {
6667
override string getName() { result = namedType.getName() }
6768

6869
/**
69-
* Returns the named type on top of this interface type
70+
* Gets the named type on top of this interface type.
7071
*/
7172
NamedType getNamedType() { result = namedType }
7273
}
@@ -87,7 +88,7 @@ module Twirp {
8788
}
8889

8990
/**
90-
* A Twirp server
91+
* A Twirp server.
9192
*/
9293
class ServiceServerType extends NamedType {
9394
ServiceServerType() {
@@ -101,22 +102,23 @@ module Twirp {
101102
}
102103

103104
/**
104-
* A Twirp function to construct a Client
105+
* A Twirp function to construct a Client.
105106
*/
106107
class ClientConstructor extends Function {
107108
ClientConstructor() {
108109
exists(ServiceClientType c |
109110
this.getName().toLowerCase() = "new" + c.getName().toLowerCase() and
110-
this.getParameter(0).getType() instanceof StringType and
111+
this.getParameterType(0) instanceof StringType and
111112
this.getParameterType(1).getName() = "HTTPClient" and
112113
this.getDeclaration().getLocation().getFile() instanceof ServicesGeneratedFile
113114
)
114115
}
115116
}
116117

117118
/**
118-
* A Twirp function to construct a Server
119-
* Its first argument should be an implementation of the service interface
119+
* A Twirp function to construct a Server.
120+
*
121+
* Its first argument should be an implementation of the service interface.
120122
*/
121123
class ServerConstructor extends Function {
122124
ServerConstructor() {
@@ -129,7 +131,7 @@ module Twirp {
129131
}
130132

131133
/**
132-
* An SSRF sink for the Client constructor
134+
* An SSRF sink for the Client constructor.
133135
*/
134136
class ClientRequestUrlAsSink extends RequestForgery::Sink {
135137
ClientRequestUrlAsSink() {
@@ -145,7 +147,7 @@ module Twirp {
145147
}
146148

147149
/**
148-
* A service handler
150+
* A service handler.
149151
*/
150152
class ServiceHandler extends Method {
151153
ServiceHandler() {

go/ql/test/library-tests/semmle/go/frameworks/Twirp/server/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ package main
22

33
import (
44
"context"
5-
"strconv"
65
"fmt"
76
"net/http"
7+
"strconv"
88
"time"
99

1010
"github.com/pwntester/go-twirp-rpc-example/rpc/notes"
@@ -27,7 +27,7 @@ func (s *notesService) CreateNote(ctx context.Context, params *notes.CreateNoteP
2727
CreatedAt: time.Now().UnixMilli(),
2828
}
2929

30-
notes.NewNotesServiceProtobufClient(params.Text, &http.Client{}) // test: ssrfSink, ssrf
30+
notes.NewNotesServiceProtobufClient(params.Text, &http.Client{}) // test: ssrfSink, ssrf
3131
notes.NewNotesServiceProtobufClient(strconv.FormatInt(int64(s.CurrentId), 10), &http.Client{}) // test: ssrfSink, !ssrf
3232

3333
s.Notes = append(s.Notes, note)

0 commit comments

Comments
 (0)