Lands with #893, which is where readCatalogRows() and reportSkippedRows() are: they are not on master yet.
Problem
A clear reads the catalog of its backend and passes over any row it cannot act on — one whose key is not the name of a tree, or whose value records a table outside the namespace this backend names its tables in. Every such row is described into a list, and the whole list goes into one logger.warn:
// readCatalogRows(): one description per row, no cap
skippedRows.add(...); // embeds the key and the recorded table name verbatim
...
// reportSkippedRows(): the whole List rendered into a single line
reportClearLine(LocalizableMessage.raw("jdbc: backend %s: %d row(s) of its catalog named nothing this clear could drop and were passed over: %s. ...",
config.getBackendId(), skippedRows.size(), skippedRows));Two things follow, and the second is the one worth acting on:
- No cap. A catalog holding thousands of such rows becomes one multi-megabyte log record.
- The values are embedded as they stand. They come out of a table, and the whole premise of this line is a catalog "written into by something other than this backend". A key or a recorded name carrying a newline splices arbitrary text into the server log where it reads as further log records — the pattern CodeQL flags as log injection.
Why it is not urgent
Nothing this version writes makes such a row: getTableName() names every table opendj_<hash> and every key is TreeName.toString(). The line exists precisely for a catalog somebody else wrote into, so its input is untrusted by construction — but reaching it takes a writer on that database.
What the fix looks like
Report the first N and a count of the rest, and put the embedded values through a sanitiser that strips control characters (or renders them escaped) before they reach the line. CachedConnection.redact() is the nearest precedent in this package for "a value that reaches a log is put through something first"; this wants the same shape, for a different reason.
Lands with #893, which is where
readCatalogRows()andreportSkippedRows()are: they are not onmasteryet.Problem
A clear reads the catalog of its backend and passes over any row it cannot act on — one whose key is not the name of a tree, or whose value records a table outside the namespace this backend names its tables in. Every such row is described into a list, and the whole list goes into one
logger.warn:Two things follow, and the second is the one worth acting on:
Why it is not urgent
Nothing this version writes makes such a row:
getTableName()names every tableopendj_<hash>and every key isTreeName.toString(). The line exists precisely for a catalog somebody else wrote into, so its input is untrusted by construction — but reaching it takes a writer on that database.What the fix looks like
Report the first N and a count of the rest, and put the embedded values through a sanitiser that strips control characters (or renders them escaped) before they reach the line.
CachedConnection.redact()is the nearest precedent in this package for "a value that reaches a log is put through something first"; this wants the same shape, for a different reason.