Skip to content

Commit 92ef176

Browse files
committed
YQ-3566 fix sql injection in create binding request (ydb-platform#7984)
1 parent 4d16b23 commit 92ef176

File tree

3 files changed

+59
-1
lines changed

3 files changed

+59
-1
lines changed

ydb/core/fq/libs/common/util.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,18 +62,24 @@ class TIssueDatabaseRemover {
6262
TString DatabasePath;
6363
};
6464

65+
void EscapeBackslashes(TString& value) {
66+
SubstGlobal(value, "\\", "\\\\");
67+
}
68+
6569
}
6670

6771
TString EscapeString(const TString& value,
6872
const TString& enclosingSeq,
6973
const TString& replaceWith) {
7074
auto escapedValue = value;
75+
EscapeBackslashes(escapedValue);
7176
SubstGlobal(escapedValue, enclosingSeq, replaceWith);
7277
return escapedValue;
7378
}
7479

7580
TString EscapeString(const TString& value, char enclosingChar) {
7681
auto escapedValue = value;
82+
EscapeBackslashes(escapedValue);
7783
SubstGlobal(escapedValue,
7884
TString{enclosingChar},
7985
TStringBuilder{} << '\\' << enclosingChar);

ydb/core/fq/libs/common/util_ut.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,19 @@ Y_UNIT_TEST_SUITE(EscapingBasics) {
2323
UNIT_ASSERT_VALUES_EQUAL(EscapeString("some_secret1", '"'), "some_secret1");
2424
UNIT_ASSERT_VALUES_EQUAL(EscapeString("some_secret1", "}+{", "[*]"), "some_secret1");
2525
UNIT_ASSERT_VALUES_EQUAL(EscapeString("some\"_\"secret1", '"'), "some\\\"_\\\"secret1");
26+
UNIT_ASSERT_VALUES_EQUAL(EscapeString("some\"_\\\"secret1", '"'), "some\\\"_\\\\\\\"secret1");
2627
UNIT_ASSERT_VALUES_EQUAL(EscapeString("some}+{_}+{secret1", "}+{", "[*]"), "some[*]_[*]secret1");
28+
UNIT_ASSERT_VALUES_EQUAL(EscapeString("some}+{\\}+{secret1", "}+{", "[*]"), "some[*]\\\\[*]secret1");
2729
}
2830

2931
Y_UNIT_TEST(EncloseAndEscapeStringShouldWork) {
3032
UNIT_ASSERT_VALUES_EQUAL(EncloseAndEscapeString("some_secret1", '"'), "\"some_secret1\"");
3133
UNIT_ASSERT_VALUES_EQUAL(EncloseAndEscapeString("some_secret1\nsome_secret2", "}+{", "[*]"), "}+{some_secret1\nsome_secret2}+{");
3234

3335
UNIT_ASSERT_VALUES_EQUAL(EncloseAndEscapeString("some\"_\"secret1", '"'), "\"some\\\"_\\\"secret1\"");
36+
UNIT_ASSERT_VALUES_EQUAL(EncloseAndEscapeString("some\"_\\\"secret1", '"'), "\"some\\\"_\\\\\\\"secret1\"");
3437
UNIT_ASSERT_VALUES_EQUAL(EncloseAndEscapeString("some_secret1}+{\n}+{some_secret2", "}+{", "[*]"), "}+{some_secret1[*]\n[*]some_secret2}+{");
38+
UNIT_ASSERT_VALUES_EQUAL(EncloseAndEscapeString("some_secret1}+{\\}+{some_secret2", "}+{", "[*]"), "}+{some_secret1[*]\\\\[*]some_secret2}+{");
3539
}
3640
}
3741

ydb/tests/fq/s3/test_bindings.py

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -632,4 +632,52 @@ def test_raw_empty_schema_binding(self, kikimr, client, unique_prefix):
632632
columns=[],
633633
check_issues=False)
634634
assert "Only one column in schema supported in raw format" in str(binding_response.issues), str(
635-
binding_response.issues)
635+
binding_response.issues)
636+
637+
@yq_all
638+
@pytest.mark.parametrize("client", [{"folder_id": "my_folder"}], indirect=True)
639+
def test_binding_with_backslash_in_location(self, s3, client):
640+
resource = boto3.resource(
641+
"s3", endpoint_url=s3.s3_url, aws_access_key_id="key", aws_secret_access_key="secret_key"
642+
)
643+
644+
bucket = resource.Bucket("backslash_bucket")
645+
bucket.create(ACL='public-read')
646+
647+
s3_client = boto3.client(
648+
"s3", endpoint_url=s3.s3_url, aws_access_key_id="key", aws_secret_access_key="secret_key"
649+
)
650+
651+
data = R'''data
652+
test'''
653+
s3_client.put_object(Body=data, Bucket='backslash_bucket', Key='\\', ContentType='text/plain')
654+
655+
connection_response = client.create_storage_connection("backslash_bucket", "backslash_bucket")
656+
657+
data_type = ydb.Column(name="data", type=ydb.Type(type_id=ydb.Type.PrimitiveTypeId.UTF8))
658+
storage_binding_name = "binding_name"
659+
client.create_object_storage_binding(
660+
name=storage_binding_name,
661+
path="\\",
662+
format="csv_with_names",
663+
connection_id=connection_response.result.connection_id,
664+
columns=[data_type],
665+
)
666+
667+
sql = fR'''
668+
SELECT *
669+
FROM bindings.{storage_binding_name};
670+
'''
671+
672+
query_id = client.create_query(
673+
"simple", sql, type=fq.QueryContent.QueryType.ANALYTICS, pg_syntax=True
674+
).result.query_id
675+
client.wait_query_status(query_id, fq.QueryMeta.COMPLETED)
676+
677+
data = client.get_result_data(query_id)
678+
result_set = data.result.result_set
679+
logging.debug(str(result_set))
680+
assert len(result_set.columns) == 1
681+
assert result_set.columns[0].name == "data"
682+
assert len(result_set.rows) == 1
683+
assert result_set.rows[0].items[0].text_value == "test"

0 commit comments

Comments
 (0)