zeroshade commented on code in PR #936:
URL: https://github.com/apache/arrow-go/pull/936#discussion_r3580663708
##########
arrow/flight/flightsql/example/sqlite_server.go:
##########
@@ -581,6 +581,15 @@ func scalarToIFace(s scalar.Scalar) (interface{}, error) {
return val.ToTime(), nil
case *scalar.DenseUnion:
return scalarToIFace(val.Value)
+ case *scalar.Dictionary:
+ value, err := val.GetEncodedValue()
Review Comment:
This decodes client-supplied dictionary indices without a lower-bound check.
`Dictionary.GetEncodedValue()` (arrow/scalar/nested.go:521) passes the raw
signed index straight to `scalar.GetScalar`, which only guards the upper bound
(`idx >= arr.Len()`, arrow/scalar/scalar.go:581) and never rejects negative
indices. So a parameter batch with a `dictionary<values=string, indices=int32>`
column whose index is e.g. `-1` reaches `arr.Value(-1)` and panics the server
instead of returning a bind error.
Since these arrive from Flight clients, please validate before decoding —
e.g. call `val.ValidateFull()` here and return the error — or add explicit
bounds checks. A follow-up hardening `scalar.GetScalar`/`GetEncodedValue` to
reject negative indices library-wide would be even better, since
`NewDictionaryArray` doesn't bounds-check either.
Worth adding tests for: a negative/out-of-range index, a null value inside
the dictionary, a non-string dictionary value type, and the prepared-statement
query path (this currently only exercises ExecuteUpdate).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
