apache / apache/arrow

[C++][FlightRPC][ODBC] SQL_ATTR_CURRENT_CATALOG (and SQL_DESC_NAME) decode a wide string with the byte-wise decoder

Open
#50,852 0 comments 0 reactions 1 assignee Claimed by @vikrantpuppala View on GitHub
Component: C++ Component: FlightRPC
Dominant language
C++
Stars
17.1k
Forks
4.3k
Avg merge
3d 13h
Merged PRs (30d)
88

Description

### Describe the bug, including details regarding any error messages, version, and platform.

In the Arrow Flight SQL ODBC driver, `ODBCConnection::SetConnectAttr` decodes
the `SQL_ATTR_CURRENT_CATALOG` value with the *wrong* string decoder when the
call arrives through a wide (Unicode / `*W`) entry point.

```cpp
// odbc_impl/odbc_connection.cc
case SQL_ATTR_CURRENT_CATALOG: {
std::string catalog;
if (is_unicode) {
SetAttributeUTF8(value, string_length, catalog); // <-- wrong
} else {
SetAttributeSQLWCHAR(value, string_length, catalog); // <-- wrong
}
...
```

`is_unicode` selects the buffer **width**, not "needs conversion":

- A unicode (`SQLSetConnectAttrW`) call hands over a **wide `SQLWCHAR` buffer**
and must be decoded with `SetAttributeSQLWCHAR`.
- A non-unicode (`SQLSetConnectAttrA`) call hands over a **byte string** and is
decoded with `SetAttributeUTF8`.

The two branches are swapped. When a wide catalog name is decoded with the
byte-wise `SetAttributeUTF8`, the wide buffer is misread: with a null-terminated
(`SQL_NTS`) length it is truncated at the first embedded NUL of the wide
encoding (e.g. UTF-16 `"odbc"` → `"o"`); with an explicit length it is stored
raw, embedded NULs and all (UTF-16 `"my_catalog"` →
`"m\0y\0_\0c\0a\0t\0a\0l\0o\0g\0"`). Either way the stored catalog is wrong.

The correct mapping is already established by the getter side: `GetStringAttribute`
(in `attribute_utils.h`) maps `is_unicode == true` to `GetAttributeSQLWCHAR`.
The setter for `SQL_ATTR_CURRENT_CATALOG` inverts it.

The same class of bug exists in `ODBCDescriptor::SetField` for `SQL_DESC_NAME`,
which unconditionally uses the byte-wise `SetAttributeUTF8` even though the
matching getter (`GetField` / `SQL_DESC_NAME`) reads the field back with
`GetAttributeSQLWCHAR` — so the field is stored wide-origin but decoded
byte-wise on the way in.

**Impact:** an application that sets a multi-character catalog through the
wide entry point gets a corrupted catalog name back, so subsequent catalog
scoping operates on the wrong (or a non-existent) catalog.

**Introduced by:**
- `SQL_ATTR_CURRENT_CATALOG`: swapped from the start in the original driver
import, GH-46522 (#40939). The getter was written against the shared
`GetStringAttribute` helper (correct); the setter open-coded the branch inline
and inverted it.
- `SQL_DESC_NAME`: originally consistent (getter and setter both byte-wise);
GH-47721 (#48050) migrated the descriptor string getters to wide
`GetAttributeSQLWCHAR` but left this setter byte-wise.

### Component(s)

C++, FlightRPC

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.