Skip to content

Commit 8e1d727

Browse files
libotonylibotony
authored
api: reject request body with null in slice (#1087)
1 parent 4ff4d5a commit 8e1d727

11 files changed

Lines changed: 117 additions & 36 deletions

File tree

‎api/accounts/accounts.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func (a *Accounts) handleCallContract(w http.ResponseWriter, req *http.Request)
196196
}
197197
var batchCallData = &BatchCallData{
198198
Clauses: Clauses{
199-
Clause{
199+
&Clause{
200200
To: addr,
201201
Value: callData.Value,
202202
Data: callData.Data,
@@ -214,10 +214,16 @@ func (a *Accounts) handleCallContract(w http.ResponseWriter, req *http.Request)
214214
}
215215

216216
func (a *Accounts) handleCallBatchCode(w http.ResponseWriter, req *http.Request) error {
217-
batchCallData := &BatchCallData{}
217+
var batchCallData BatchCallData
218218
if err := utils.ParseJSON(req.Body, &batchCallData); err != nil {
219219
return utils.BadRequest(errors.WithMessage(err, "body"))
220220
}
221+
// reject null element in clauses, {} will be unmarshaled to default value and will be accepted/handled by the runtime
222+
for i, clause := range batchCallData.Clauses {
223+
if clause == nil {
224+
return utils.BadRequest(fmt.Errorf("clauses[%d]: null not allowed", i))
225+
}
226+
}
221227
revision, err := utils.ParseRevision(req.URL.Query().Get("revision"), true)
222228
if err != nil {
223229
return utils.BadRequest(errors.WithMessage(err, "revision"))
@@ -229,7 +235,7 @@ func (a *Accounts) handleCallBatchCode(w http.ResponseWriter, req *http.Request)
229235
}
230236
return err
231237
}
232-
results, err := a.batchCall(req.Context(), batchCallData, summary.Header, st)
238+
results, err := a.batchCall(req.Context(), &batchCallData, summary.Header, st)
233239
if err != nil {
234240
return err
235241
}

‎api/accounts/accounts_test.go‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ func TestAccount(t *testing.T) {
121121
"callContractWithNonExistingRevision": callContractWithNonExistingRevision,
122122
"batchCall": batchCall,
123123
"batchCallWithNonExistingRevision": batchCallWithNonExistingRevision,
124+
"batchCallWithNullClause": batchCallWithNullClause,
124125
} {
125126
t.Run(name, tt)
126127
}
@@ -433,12 +434,12 @@ func batchCall(t *testing.T) {
433434
// Request body is not a valid BatchCallData
434435
badBody := &accounts.BatchCallData{
435436
Clauses: accounts.Clauses{
436-
accounts.Clause{
437+
&accounts.Clause{
437438
To: &contractAddr,
438439
Data: "data1",
439440
Value: nil,
440441
},
441-
accounts.Clause{
442+
&accounts.Clause{
442443
To: &contractAddr,
443444
Data: "data2",
444445
Value: nil,
@@ -478,12 +479,12 @@ func batchCall(t *testing.T) {
478479
}
479480
reqBody := &accounts.BatchCallData{
480481
Clauses: accounts.Clauses{
481-
accounts.Clause{
482+
&accounts.Clause{
482483
To: &contractAddr,
483484
Data: hexutil.Encode(input),
484485
Value: nil,
485486
},
486-
accounts.Clause{
487+
&accounts.Clause{
487488
To: &contractAddr,
488489
Data: hexutil.Encode(input),
489490
Value: nil,
@@ -556,3 +557,23 @@ func batchCallWithNonExistingRevision(t *testing.T) {
556557
assert.Equal(t, http.StatusBadRequest, statusCode, "bad revision")
557558
assert.Equal(t, "revision: leveldb: not found\n", string(res), "revision not found")
558559
}
560+
561+
func batchCallWithNullClause(t *testing.T) {
562+
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/accounts/*", []byte("{\"clauses\": [null]}"))
563+
assert.NoError(t, err)
564+
assert.Equal(t, http.StatusBadRequest, statusCode, "null clause")
565+
assert.Equal(t, "clauses[0]: null not allowed\n", string(res), "null clause")
566+
567+
res, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/accounts/*", []byte("{\"clauses\": [{}, null]}"))
568+
assert.NoError(t, err)
569+
assert.Equal(t, http.StatusBadRequest, statusCode, "null clause")
570+
assert.Equal(t, "clauses[1]: null not allowed\n", string(res), "null clause")
571+
572+
_, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/accounts/*", []byte("{\"clauses\":null }"))
573+
assert.NoError(t, err)
574+
assert.Equal(t, http.StatusOK, statusCode, "null clause")
575+
576+
_, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/accounts/*", []byte("{\"clauses\":[] }"))
577+
assert.NoError(t, err)
578+
assert.Equal(t, http.StatusOK, statusCode, "null clause")
579+
}

‎api/accounts/types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ type Clause struct {
9696
}
9797

9898
// Clauses array of clauses.
99-
type Clauses []Clause
99+
type Clauses []*Clause
100100

101101
// BatchCallData executes a batch of codes
102102
type BatchCallData struct {

‎api/events/events.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ func (e *Events) handleFilter(w http.ResponseWriter, req *http.Request) error {
6464
if filter.Range != nil && filter.Range.From != nil && filter.Range.To != nil && *filter.Range.From > *filter.Range.To {
6565
return utils.BadRequest(fmt.Errorf("filter.Range.To must be greater than or equal to filter.Range.From"))
6666
}
67+
// reject null element in CriteriaSet, {} will be unmarshaled to default value and will be accepted/handled by the filter engine
68+
for i, criterion := range filter.CriteriaSet {
69+
if criterion == nil {
70+
return utils.BadRequest(fmt.Errorf("criteriaSet[%d]: null not allowed", i))
71+
}
72+
}
6773
if filter.Options == nil {
6874
// if filter.Options is nil, set to the default limit +1
6975
// to detect whether there are more logs than the default limit

‎api/events/events_test.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,26 @@ func TestZeroFrom(t *testing.T) {
185185
assert.NotEmpty(t, tLogs)
186186
}
187187

188+
func TestNullCriteriaSet(t *testing.T) {
189+
initEventServer(t, defaultLogLimit)
190+
defer ts.Close()
191+
192+
tclient = thorclient.New(ts.URL)
193+
_, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/event", []byte(`{"criteriaSet": null}`))
194+
require.NoError(t, err)
195+
assert.Equal(t, http.StatusOK, statusCode)
196+
197+
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/event", []byte(`{"criteriaSet": [null]}`))
198+
require.NoError(t, err)
199+
assert.Equal(t, http.StatusBadRequest, statusCode)
200+
assert.Equal(t, "criteriaSet[0]: null not allowed\n", string(res), "null criteriaSet")
201+
202+
res, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/logs/event", []byte(`{"criteriaSet": [{}, null]}`))
203+
require.NoError(t, err)
204+
assert.Equal(t, http.StatusBadRequest, statusCode)
205+
assert.Equal(t, "criteriaSet[1]: null not allowed\n", string(res), "null criteriaSet")
206+
}
207+
188208
// Test functions
189209
func testEventsBadRequest(t *testing.T) {
190210
badBody := []byte{0x00, 0x01, 0x02}

‎api/events/types.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,6 @@ type LogMeta struct {
2424
LogIndex *uint32 `json:"logIndex,omitempty"`
2525
}
2626

27-
type TopicSet struct {
28-
Topic0 *thor.Bytes32 `json:"topic0"`
29-
Topic1 *thor.Bytes32 `json:"topic1"`
30-
Topic2 *thor.Bytes32 `json:"topic2"`
31-
Topic3 *thor.Bytes32 `json:"topic3"`
32-
Topic4 *thor.Bytes32 `json:"topic4"`
33-
}
34-
3527
// FilteredEvent only comes from one contract
3628
type FilteredEvent struct {
3729
Address thor.Address `json:"address"`
@@ -69,6 +61,14 @@ func convertEvent(event *logdb.Event, addIndexes bool) *FilteredEvent {
6961
return fe
7062
}
7163

64+
type TopicSet struct {
65+
Topic0 *thor.Bytes32 `json:"topic0"`
66+
Topic1 *thor.Bytes32 `json:"topic1"`
67+
Topic2 *thor.Bytes32 `json:"topic2"`
68+
Topic3 *thor.Bytes32 `json:"topic3"`
69+
Topic4 *thor.Bytes32 `json:"topic4"`
70+
}
71+
7272
type EventCriteria struct {
7373
Address *thor.Address `json:"address"`
7474
TopicSet

‎api/transfers/transfers.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ func (t *Transfers) handleFilterTransferLogs(w http.ResponseWriter, req *http.Re
7373
if filter.Range != nil && filter.Range.From != nil && filter.Range.To != nil && *filter.Range.From > *filter.Range.To {
7474
return utils.BadRequest(fmt.Errorf("filter.Range.To must be greater than or equal to filter.Range.From"))
7575
}
76+
// reject null element in CriteriaSet, {} will be unmarshaled to default value and will be accepted/handled by the filter engine
77+
for i, criterion := range filter.CriteriaSet {
78+
if criterion == nil {
79+
return utils.BadRequest(fmt.Errorf("criteriaSet[%d]: null not allowed", i))
80+
}
81+
}
7682
if filter.Options == nil {
7783
// if filter.Options is nil, set to the default limit +1
7884
// to detect whether there are more logs than the default limit

‎api/transfers/transfers_test.go‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,20 +69,20 @@ func TestOption(t *testing.T) {
6969
Order: logdb.DESC,
7070
}
7171

72-
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfers", filter)
72+
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", filter)
7373
require.NoError(t, err)
7474
assert.Equal(t, "options.limit exceeds the maximum allowed value of 5", strings.Trim(string(res), "\n"))
7575
assert.Equal(t, http.StatusForbidden, statusCode)
7676

7777
filter.Options.Limit = 5
78-
_, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/logs/transfers", filter)
78+
_, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", filter)
7979
require.NoError(t, err)
8080
assert.Equal(t, http.StatusOK, statusCode)
8181

8282
// with nil options, should use default limit, when the filtered lower
8383
// or equal to the limit, should return the filtered transfers
8484
filter.Options = nil
85-
res, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/logs/transfers", filter)
85+
res, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", filter)
8686
require.NoError(t, err)
8787
assert.Equal(t, http.StatusOK, statusCode)
8888
var tLogs []*events.FilteredEvent
@@ -94,7 +94,7 @@ func TestOption(t *testing.T) {
9494

9595
// when the filtered transfers exceed the limit, should return the forbidden
9696
insertBlocks(t, db, 6)
97-
res, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/logs/transfers", filter)
97+
res, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", filter)
9898
require.NoError(t, err)
9999
assert.Equal(t, http.StatusForbidden, statusCode)
100100
assert.Equal(t, "the number of filtered logs exceeds the maximum allowed value of 5, please use pagination", strings.Trim(string(res), "\n"))
@@ -133,7 +133,7 @@ func TestOptionalData(t *testing.T) {
133133
Order: logdb.DESC,
134134
}
135135

136-
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfers", filter)
136+
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", filter)
137137
assert.NoError(t, err)
138138
assert.Equal(t, http.StatusOK, statusCode)
139139
var tLogs []*transfers.FilteredTransfer
@@ -151,11 +151,33 @@ func TestOptionalData(t *testing.T) {
151151
}
152152
}
153153

154+
func TestNullCriteriaSet(t *testing.T) {
155+
db := createDb(t)
156+
initTransferServer(t, db, defaultLogLimit)
157+
defer ts.Close()
158+
tclient = thorclient.New(ts.URL)
159+
160+
_, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", []byte(`{"criteriaSet": null}`))
161+
require.NoError(t, err)
162+
163+
assert.Equal(t, http.StatusOK, statusCode)
164+
165+
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", []byte(`{"criteriaSet": [null]}`))
166+
require.NoError(t, err)
167+
assert.Equal(t, http.StatusBadRequest, statusCode)
168+
assert.Equal(t, "criteriaSet[0]: null not allowed\n", string(res), "null criteriaSet")
169+
170+
res, statusCode, err = tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", []byte(`{"criteriaSet": [{}, null]}`))
171+
require.NoError(t, err)
172+
assert.Equal(t, http.StatusBadRequest, statusCode)
173+
assert.Equal(t, "criteriaSet[1]: null not allowed\n", string(res), "null criteriaSet")
174+
}
175+
154176
// Test functions
155177
func testTransferBadRequest(t *testing.T) {
156178
badBody := []byte{0x00, 0x01, 0x02}
157179

158-
_, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfers", badBody)
180+
_, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", badBody)
159181
require.NoError(t, err)
160182
assert.Equal(t, http.StatusBadRequest, statusCode)
161183
}
@@ -168,7 +190,7 @@ func testTransferWithEmptyDb(t *testing.T) {
168190
Order: logdb.DESC,
169191
}
170192

171-
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfers", emptyFilter)
193+
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", emptyFilter)
172194
require.NoError(t, err)
173195
var tLogs []*transfers.FilteredTransfer
174196
if err := json.Unmarshal(res, &tLogs); err != nil {
@@ -187,7 +209,7 @@ func testTransferWithBlocks(t *testing.T, expectedBlocks int) {
187209
Order: logdb.DESC,
188210
}
189211

190-
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfers", emptyFilter)
212+
res, statusCode, err := tclient.RawHTTPClient().RawHTTPPost("/logs/transfer", emptyFilter)
191213
require.NoError(t, err)
192214
var tLogs []*transfers.FilteredTransfer
193215
if err := json.Unmarshal(res, &tLogs); err != nil {
@@ -226,7 +248,7 @@ func initTransferServer(t *testing.T, logDb *logdb.LogDB, limit uint64) {
226248
require.NoError(t, err)
227249

228250
router := mux.NewRouter()
229-
transfers.New(thorChain.Repo(), logDb, limit).Mount(router, "/logs/transfers")
251+
transfers.New(thorChain.Repo(), logDb, limit).Mount(router, "/logs/transfer")
230252

231253
ts = httptest.NewServer(router)
232254
}

‎api/transfers/types.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@ type FilteredTransfer struct {
3030
Meta LogMeta `json:"meta"`
3131
}
3232

33+
type TransferFilter struct {
34+
CriteriaSet []*logdb.TransferCriteria
35+
Range *events.Range
36+
Options *events.Options
37+
Order logdb.Order //default asc
38+
}
39+
3340
func convertTransfer(transfer *logdb.Transfer, addIndexes bool) *FilteredTransfer {
3441
v := math.HexOrDecimal256(*transfer.Amount)
3542
ft := &FilteredTransfer{
@@ -53,10 +60,3 @@ func convertTransfer(transfer *logdb.Transfer, addIndexes bool) *FilteredTransfe
5360

5461
return ft
5562
}
56-
57-
type TransferFilter struct {
58-
CriteriaSet []*logdb.TransferCriteria
59-
Range *events.Range
60-
Options *events.Options
61-
Order logdb.Order //default asc
62-
}

‎thorclient/api_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,12 +200,12 @@ func testAccountEndpoint(t *testing.T, _ *testchain.Chain, ts *httptest.Server)
200200
value := math.HexOrDecimal256(*big.NewInt(1))
201201
payload := &accounts.BatchCallData{
202202
Clauses: accounts.Clauses{
203-
accounts.Clause{
203+
&accounts.Clause{
204204
To: &address1,
205205
Value: nil,
206206
Data: "0x",
207207
},
208-
accounts.Clause{
208+
&accounts.Clause{
209209
To: &address2,
210210
Value: &value,
211211
Data: "0x",

0 commit comments

Comments
 (0)