fix(security): parameterize readByUUID to close CQL injection (H1) - #146
Open
pallakartheekreddy wants to merge 1 commit into
Open
fix(security): parameterize readByUUID to close CQL injection (H1)#146pallakartheekreddy wants to merge 1 commit into
pallakartheekreddy wants to merge 1 commit into
Conversation
CassandraStore.readByUUID built CQL by raw string concatenation: "select * from " + keyspace+"."+table + " where " + key + "=" + value + ";" Its only caller, QRCodesStore.read, passed the un-sanitized :processid path param from GET /dialcode/v4/batch/read/:processid straight in, so a crafted processid injected CQL. - readByUUID now builds the query with QueryBuilder.eq(key, value) so the value is bound through its TypeCodec instead of concatenated (mirrors the sibling read()). Signature tightened to java.util.UUID (processid is a uuid column; single caller). - QRCodesStore.read parses the path param via UUID.fromString before the DB call, so any malformed id (including injection payloads) is rejected up front. Response for valid UUIDs is unchanged; malformed input still surfaces as ERR_QRCODES_BATCH_INFO (not-found), preserving existing client behaviour. No schema change. Verified: dial-service test-compile under JDK 11 (both classes recompiled, exit 0). Implements H1 (P0) from implementation-designs/sunbird-dial-service.md.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the CQL injection (H1, P0) in
implementation-designs/sunbird-dial-service.md.The vulnerability
CassandraStore.readByUUIDbuilt CQL by raw string concatenation:Its only caller,
QRCodesStore.read, passed the un-sanitized:processidpath param fromGET /dialcode/v4/batch/read/:processidstraight intovalue, so a craftedprocessidinjected CQL.The fix
readByUUIDnow builds the query withQueryBuilder.eq(key, value)— the value is bound through itsTypeCodecinstead of concatenated (mirrors the siblingread()already in this class). Signature tightened tojava.util.UUID(processidis auuidcolumn; single caller).QRCodesStore.readparses the path param viaUUID.fromStringbefore the DB call, so any malformed id — including an injection payload likex' OR 1=1--— is rejected up front.keyis a compile-time enum name (DialCodeEnum.processid.name()), never user input.Backward compatibility
Response for valid UUIDs is unchanged. Malformed input still surfaces as
ERR_QRCODES_BATCH_INFO(not-found) via the existing catch, preserving current client behaviour. No DB/schema change.Verification
mvn test-compileunder JDK 11 — exit 0; bothCassandraStoreandQRCodesStorerecompiled.Note: two pre-existing tracked deletions under
build/in the working tree are unrelated and excluded from this branch.