Dangerous serialization in slices.Strings, broken behavior

[hdm]

Description

Use of slices.String as a column type should be considered dangerous and unreliable as currently implemented. The serialization/deserialization process joins on comma and splits on comma, with no regard for string data containing commas or any other SQL-relevant fields. This can result in SQL injection and generally breaks use of string array column types. The serialization issue can be partially solved by the caller by escaping values in advance, but the deserialization side requires a code change to make this feature usable.

Steps to Reproduce the Problem

This example assumes postgresql.

1. Define a text[] column in a fizz migration:

	t.Column("names", "text[]", {"null": true})

2. Set the column to a slices.String type in the model definition:

	Names            slices.String `json:"names" db:"names"`

3. Try setting this column to a string array containing values that contain double quotes or commas:

m := models.MyModel{Names: []string{"This has a comma,", "This has a double quote\""}}
m.Save()

4. Observe havoc (potential SQL injection among other badness):

time="2019-01-27T21:03:28-06:00" level=info msg="error updating modelname pq: malformed array literal: \"{This has a comma,,This has a double quote\"}\" ({\"id\":\"c099cfb6-42ac-4d7c-925a-6e...

Expected Behavior

Escaping array literals is complicated in postgresql and the safest path is to use the ARRAY[] syntax instead. For the value []string{"This has a comma,", "This has a double quote\"","Also a single'"} the syntax would look like:

set names = ARRAY['This has a comma,', 'This has a double quote"', 'Also a single''']

Single quotes in values must be escaped by doubling them up (or using the posix \E syntax and backslashes).

Deserialization is still a challenge, as the case above returns this literal value:

 {"This has a comma,","This has a double quote\"","Also a single'"}

Values can also be unquoted and it looks like the only way to do this correctly is to use the pg.Array parser:

• https://github.com/go-pg/pg/blob/3f0c886d21f634f2189cd3c619e4b617eea08753/types/array_parser.go#L59

Actual Behavior

Arrays would be serialized and deserialized safely.

Info

pop v4.9.4 w/buffalo 0.14.0-beta2

[hdm]

It looks like the best workaround at the moment is moving to jsonb and using a json array instead.