Skip to content

Commit a10c255

Browse files
kotharironakjainnidhi703
kotharironak
and
jainnidhi703
authored
Fixing SQLi Vulnerability (#30)
* Fixing SQLI Vulnerability * Adding params test cases * cleaning imports * PR review changes * Moving to Object Params * -wip * wip * Adds cast logic, test cases, and assumptions * Fixed integration test * Addrese the change for IN operator and handle case of List Co-authored-by: Nidhi <jainnidhi703@gmail.com>
1 parent 316cbfb commit a10c255

File tree

7 files changed

+710
-164
lines changed

7 files changed

+710
-164
lines changed

‎document-store/src/integrationTest/java/org/hypertrace/core/documentstore/mongo/MongoDocStoreTest.java‎

Lines changed: 189 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,18 @@
3131
import java.util.Objects;
3232
import java.util.Set;
3333
import java.util.stream.Collectors;
34+
import org.apache.commons.lang3.tuple.ImmutablePair;
3435
import org.hypertrace.core.documentstore.Collection;
3536
import org.hypertrace.core.documentstore.Datastore;
3637
import org.hypertrace.core.documentstore.DatastoreProvider;
3738
import org.hypertrace.core.documentstore.Document;
3839
import org.hypertrace.core.documentstore.Filter;
40+
import org.hypertrace.core.documentstore.Filter.Op;
3941
import org.hypertrace.core.documentstore.JSONDocument;
4042
import org.hypertrace.core.documentstore.Key;
4143
import org.hypertrace.core.documentstore.Query;
4244
import org.hypertrace.core.documentstore.SingleValueKey;
45+
import org.hypertrace.core.documentstore.utils.Utils;
4346
import org.junit.jupiter.api.AfterEach;
4447
import org.junit.jupiter.api.Assertions;
4548
import org.junit.jupiter.api.BeforeAll;
@@ -102,7 +105,7 @@ public void testCollections() {
102105
public void testIgnoreCaseLikeQuery() throws IOException {
103106
long now = Instant.now().toEpochMilli();
104107
Collection collection = datastore.getCollection(COLLECTION_NAME);
105-
collection.upsert(new SingleValueKey("default", "testKey"), createDocument("name", "Bob"));
108+
collection.upsert(new SingleValueKey("default", "testKey"), Utils.createDocument("name", "Bob"));
106109

107110
String[] ignoreCaseSearchValues = {"Bob", "bob", "BOB", "bOB", "BO", "bO", "Ob", "OB"};
108111

@@ -126,13 +129,13 @@ public void testIgnoreCaseLikeQuery() throws IOException {
126129
@Test
127130
public void testTotalWithQuery() throws IOException {
128131
Collection collection = datastore.getCollection(COLLECTION_NAME);
129-
collection.upsert(new SingleValueKey("default", "testKey1"), createDocument("name", "Bob"));
130-
collection.upsert(new SingleValueKey("default", "testKey2"), createDocument("name", "Alice"));
131-
collection.upsert(new SingleValueKey("default", "testKey3"), createDocument("name", "Alice"));
132-
collection.upsert(new SingleValueKey("default", "testKey4"), createDocument("name", "Bob"));
133-
collection.upsert(new SingleValueKey("default", "testKey5"), createDocument("name", "Alice"));
132+
collection.upsert(new SingleValueKey("default", "testKey1"), Utils.createDocument("name", "Bob"));
133+
collection.upsert(new SingleValueKey("default", "testKey2"), Utils.createDocument("name", "Alice"));
134+
collection.upsert(new SingleValueKey("default", "testKey3"), Utils.createDocument("name", "Alice"));
135+
collection.upsert(new SingleValueKey("default", "testKey4"), Utils.createDocument("name", "Bob"));
136+
collection.upsert(new SingleValueKey("default", "testKey5"), Utils.createDocument("name", "Alice"));
134137
collection.upsert(
135-
new SingleValueKey("default", "testKey6"), createDocument("email", "bob@example.com"));
138+
new SingleValueKey("default", "testKey6"), Utils.createDocument("email", "bob@example.com"));
136139

137140
{
138141
// empty query returns all the documents
@@ -158,11 +161,11 @@ public void testTotalWithQuery() throws IOException {
158161
@Test
159162
public void testOffsetAndLimit() throws IOException {
160163
Collection collection = datastore.getCollection(COLLECTION_NAME);
161-
collection.upsert(new SingleValueKey("default", "testKey1"), createDocument("foo1", "bar1"));
162-
collection.upsert(new SingleValueKey("default", "testKey2"), createDocument("foo2", "bar2"));
163-
collection.upsert(new SingleValueKey("default", "testKey3"), createDocument("foo3", "bar3"));
164-
collection.upsert(new SingleValueKey("default", "testKey4"), createDocument("foo4", "bar4"));
165-
collection.upsert(new SingleValueKey("default", "testKey5"), createDocument("foo5", "bar5"));
164+
collection.upsert(new SingleValueKey("default", "testKey1"), Utils.createDocument("foo1", "bar1"));
165+
collection.upsert(new SingleValueKey("default", "testKey2"), Utils.createDocument("foo2", "bar2"));
166+
collection.upsert(new SingleValueKey("default", "testKey3"), Utils.createDocument("foo3", "bar3"));
167+
collection.upsert(new SingleValueKey("default", "testKey4"), Utils.createDocument("foo4", "bar4"));
168+
collection.upsert(new SingleValueKey("default", "testKey5"), Utils.createDocument("foo5", "bar5"));
166169

167170
// Querying 5 times, to make sure the order of results is maintained with offset + limit
168171
for (int i = 0; i < 5; i++) {
@@ -362,8 +365,8 @@ public void testSubDocumentDelete() throws IOException {
362365
public void testSelectAll() throws IOException {
363366
datastore.createCollection(COLLECTION_NAME, null);
364367
Collection collection = datastore.getCollection(COLLECTION_NAME);
365-
collection.upsert(new SingleValueKey("default", "testKey1"), createDocument("testKey1", "abc1"));
366-
collection.upsert(new SingleValueKey("default", "testKey2"), createDocument("testKey2", "abc2"));
368+
collection.upsert(new SingleValueKey("default", "testKey1"), Utils.createDocument("testKey1", "abc1"));
369+
collection.upsert(new SingleValueKey("default", "testKey2"), Utils.createDocument("testKey2", "abc2"));
367370
assertEquals(2, collection.count());
368371
Iterator<Document> iterator = collection.search(new Query());
369372
List<Document> documents = new ArrayList<>();
@@ -381,8 +384,8 @@ public void testSelectAll() throws IOException {
381384
public void testSelections() throws IOException {
382385
datastore.createCollection(COLLECTION_NAME, null);
383386
Collection collection = datastore.getCollection(COLLECTION_NAME);
384-
collection.upsert(new SingleValueKey("default", "testKey1"), createDocument("testKey1", "abc1"));
385-
collection.upsert(new SingleValueKey("default", "testKey2"), createDocument("testKey2", "abc2"));
387+
collection.upsert(new SingleValueKey("default", "testKey1"), Utils.createDocument("testKey1", "abc1"));
388+
collection.upsert(new SingleValueKey("default", "testKey2"), Utils.createDocument("testKey2", "abc2"));
386389
assertEquals(2, collection.count());
387390
Query query = new Query();
388391
query.addSelection("testKey1");
@@ -412,8 +415,8 @@ public void testBulkUpsert() {
412415
datastore.createCollection(COLLECTION_NAME, null);
413416
Collection collection = datastore.getCollection(COLLECTION_NAME);
414417
Map<Key, Document> documentMap = Map.of(
415-
new SingleValueKey("default", "testKey1"), createDocument("testKey1", "abc1"),
416-
new SingleValueKey("default", "testKey2"), createDocument("testKey2", "abc2")
418+
new SingleValueKey("default", "testKey1"), Utils.createDocument("testKey1", "abc1"),
419+
new SingleValueKey("default", "testKey2"), Utils.createDocument("testKey2", "abc2")
417420
);
418421

419422
assertTrue(collection.bulkUpsert(documentMap));
@@ -430,13 +433,177 @@ public void testBulkUpsert() {
430433
assertEquals(1, collection.count());
431434
}
432435

436+
/**
437+
* This is an example where same field is having different type values.
438+
* e.g size field has boolean, numeric and string values.
439+
* This is a valid scenario for document store, and works fine with mongodb.
440+
* However, there is currently limitation on postgres as document store implementation
441+
* using jsonb, and it won't work.
442+
* */
443+
@Test
444+
public void testWithDifferentDataTypes() throws IOException {
445+
datastore.createCollection(COLLECTION_NAME, null);
446+
Collection collection = datastore.getCollection(COLLECTION_NAME);
447+
448+
// size field with integer value
449+
collection.upsert(new SingleValueKey("default", "testKey1"),
450+
Utils.createDocument(
451+
ImmutablePair.of("id", "testKey1"),
452+
ImmutablePair.of("name", "abc1"),
453+
ImmutablePair.of("size", -10))
454+
);
455+
collection.upsert(new SingleValueKey("default", "testKey2"),
456+
Utils.createDocument(
457+
ImmutablePair.of("id", "testKey2"),
458+
ImmutablePair.of("name", "abc2"),
459+
ImmutablePair.of("size", -20))
460+
);
461+
462+
// size field with string value
463+
collection.upsert(new SingleValueKey("default", "testKey3"),
464+
Utils.createDocument(
465+
ImmutablePair.of("id", "testKey3"),
466+
ImmutablePair.of("name", "abc3"),
467+
ImmutablePair.of("size", false))
468+
);
469+
collection.upsert(new SingleValueKey("default", "testKey4"),
470+
Utils.createDocument(
471+
ImmutablePair.of("id", "testKey4"),
472+
ImmutablePair.of("name", "abc4"),
473+
ImmutablePair.of("size", true))
474+
);
475+
476+
// size field with boolean value
477+
collection.upsert(new SingleValueKey("default", "testKey5"),
478+
Utils.createDocument(
479+
ImmutablePair.of("id", "testKey5"),
480+
ImmutablePair.of("name", "abc5"),
481+
ImmutablePair.of("size", "10"))
482+
);
483+
collection.upsert(new SingleValueKey("default", "testKey6"),
484+
Utils.createDocument(
485+
ImmutablePair.of("id", "testKey6"),
486+
ImmutablePair.of("name", "abc6"),
487+
ImmutablePair.of("size", "20"))
488+
);
489+
490+
// query for size field with integer value
491+
Query queryGt = new Query();
492+
Filter filterGt = new Filter(Op.GT, "size", -30);
493+
queryGt.setFilter(filterGt);
494+
Iterator<Document> results = collection.search(queryGt);
495+
List<Document> documents = new ArrayList<>();
496+
while (results.hasNext()) {
497+
documents.add(results.next());
498+
}
499+
Assertions.assertEquals(2,documents.size());
500+
501+
// query for size field with string value
502+
Query queryGtStr = new Query();
503+
Filter filterGtStr = new Filter(Op.GT, "size", "1");
504+
queryGtStr.setFilter(filterGtStr);
505+
Iterator<Document> resultsGtStr = collection.search(queryGtStr);
506+
List<Document> documentsGtStr = new ArrayList<>();
507+
while (resultsGtStr.hasNext()) {
508+
documentsGtStr.add(resultsGtStr.next());
509+
}
510+
Assertions.assertEquals(2, documentsGtStr.size());
511+
512+
// query for size field with boolean value
513+
Query queryGtBool = new Query();
514+
Filter filterGtBool = new Filter(Op.GT, "size", false);
515+
queryGtStr.setFilter(filterGtBool);
516+
Iterator<Document> resultsGtBool = collection.search(queryGtStr);
517+
List<Document> documentsGtBool = new ArrayList<>();
518+
while (resultsGtBool.hasNext()) {
519+
documentsGtBool.add(resultsGtBool.next());
520+
}
521+
Assertions.assertEquals(1, documentsGtBool.size());
522+
523+
datastore.deleteCollection(COLLECTION_NAME);
524+
}
525+
526+
@Test
527+
public void testWithDifferentFieldTypes() throws IOException {
528+
datastore.createCollection(COLLECTION_NAME, null);
529+
Collection collection = datastore.getCollection(COLLECTION_NAME);
530+
531+
// size field with integer value, isCostly boolean field
532+
collection.upsert(new SingleValueKey("default", "testKey1"),
533+
Utils.createDocument(
534+
ImmutablePair.of("id", "testKey1"),
535+
ImmutablePair.of("name", "abc1"),
536+
ImmutablePair.of("size", -10),
537+
ImmutablePair.of("isCostly", false))
538+
);
539+
540+
collection.upsert(new SingleValueKey("default", "testKey2"),
541+
Utils.createDocument(
542+
ImmutablePair.of("id", "testKey2"),
543+
ImmutablePair.of("name", "abc2"),
544+
ImmutablePair.of("size", -20),
545+
ImmutablePair.of("isCostly", false))
546+
);
547+
548+
collection.upsert(new SingleValueKey("default", "testKey3"),
549+
Utils.createDocument(
550+
ImmutablePair.of("id", "testKey3"),
551+
ImmutablePair.of("name", "abc3"),
552+
ImmutablePair.of("size", 5),
553+
ImmutablePair.of("isCostly", true))
554+
);
555+
556+
collection.upsert(new SingleValueKey("default", "testKey4"),
557+
Utils.createDocument(
558+
ImmutablePair.of("id", "testKey4"),
559+
ImmutablePair.of("name", "abc4"),
560+
ImmutablePair.of("size", 10),
561+
ImmutablePair.of("isCostly", true))
562+
);
563+
564+
// query field having int type
565+
Query queryNumericField = new Query();
566+
Filter filter = new Filter(Op.GT, "size", -30);
567+
queryNumericField.setFilter(filter);
568+
Iterator<Document> results = collection.search(queryNumericField);
569+
List<Document> documents = new ArrayList<>();
570+
while (results.hasNext()) {
571+
documents.add(results.next());
572+
}
573+
Assertions.assertEquals(4, documents.size());
574+
575+
// query field having boolean field
576+
Query queryBooleanField = new Query();
577+
filter = new Filter(Op.GT, "isCostly", false);
578+
queryBooleanField.setFilter(filter);
579+
results = collection.search(queryBooleanField);
580+
documents = new ArrayList<>();
581+
while (results.hasNext()) {
582+
documents.add(results.next());
583+
}
584+
Assertions.assertEquals(2, documents.size());
585+
586+
// query string field
587+
Query queryStringField = new Query();
588+
filter = new Filter(Op.GT, "name", "abc1");
589+
queryStringField.setFilter(filter);
590+
results = collection.search(queryBooleanField);
591+
documents = new ArrayList<>();
592+
while (results.hasNext()) {
593+
documents.add(results.next());
594+
}
595+
Assertions.assertEquals(2, documents.size());
596+
597+
datastore.deleteCollection(COLLECTION_NAME);
598+
}
599+
433600
@Test
434601
public void testReturnAndBulkUpsert() throws IOException {
435602
datastore.createCollection(COLLECTION_NAME, null);
436603
Collection collection = datastore.getCollection(COLLECTION_NAME);
437604
Map<Key, Document> documentMapV1 = Map.of(
438-
new SingleValueKey("default", "testKey1"), createDocument("id", "1", "testKey1", "abc-v1"),
439-
new SingleValueKey("default", "testKey2"), createDocument("id", "2", "testKey2", "xyz-v1")
605+
new SingleValueKey("default", "testKey1"), Utils.createDocument("id", "1", "testKey1", "abc-v1"),
606+
new SingleValueKey("default", "testKey2"), Utils.createDocument("id", "2", "testKey2", "xyz-v1")
440607
);
441608

442609
Iterator<Document> iterator = collection.bulkUpsertAndReturnOlderDocuments(documentMapV1);
@@ -445,8 +612,8 @@ public void testReturnAndBulkUpsert() throws IOException {
445612

446613
// Add more details to the document and bulk upsert again.
447614
Map<Key, Document> documentMapV2 = Map.of(
448-
new SingleValueKey("default", "testKey1"), createDocument("id", "1", "testKey1", "abc-v2"),
449-
new SingleValueKey("default", "testKey2"), createDocument("id", "2", "testKey2", "xyz-v2")
615+
new SingleValueKey("default", "testKey1"), Utils.createDocument("id", "1", "testKey1", "abc-v2"),
616+
new SingleValueKey("default", "testKey2"), Utils.createDocument("id", "2", "testKey2", "xyz-v2")
450617
);
451618
iterator = collection.bulkUpsertAndReturnOlderDocuments(documentMapV2);
452619
assertEquals(2, collection.count());
@@ -534,18 +701,4 @@ public void testLike() {
534701
}
535702
assertEquals(1, results.size());
536703
}
537-
538-
private Document createDocument(String ...keys) {
539-
ObjectNode objectNode = OBJECT_MAPPER.createObjectNode();
540-
for (int i = 0; i < keys.length - 1; i++) {
541-
objectNode.put(keys[i], keys[i + 1]);
542-
}
543-
return new JSONDocument(objectNode);
544-
}
545-
546-
private Document createDocument(String key, String value) {
547-
ObjectNode objectNode = OBJECT_MAPPER.createObjectNode();
548-
objectNode.put(key, value);
549-
return new JSONDocument(objectNode);
550-
}
551704
}

0 commit comments

Comments
 (0)