MariaDB Connector/ODBC: un lote fallido deja ArrayOffset sin reiniciar y el siguiente lee fuera de límites

Resumen

El fallo está en mariadb-connector-odbc, la librería cliente ODBC de MariaDB (analizada en la rama master, commit 4e175d9). Cuando una aplicación ejecuta un INSERT con un array de parámetros (varias filas en un solo SQLExecute), el conector recorre las filas y lleva la cuenta en el campo MADB_Stmt::ArrayOffset. Ese contador solo vuelve a cero en un sitio: MADB_Stmt::AfterExecute().

El problema es que, si el servidor rechaza una de las filas (por ejemplo, por una clave primaria duplicada), el driver lanza una excepción C++ que sale de MADB_StmtExecute sin pasar por AfterExecute(). La aplicación recibe un SQL_ERROR normal y un handle que parece reutilizable, pero con ArrayOffset apuntando todavía a la fila donde se quedó. El siguiente lote sobre ese mismo handle usa ese desplazamiento para indexar los arrays de la aplicación y lee fuera de sus límites.

El investigador lo demostró como lectura fuera de límites (heap-buffer-overflow de lectura bajo AddressSanitizer). No encontró escritura fuera de límites, y comprobó que MADB_SetStatusArray indexa desde cero.

Dónde está el fallo

En driver/ma_statement.cpp (líneas 1281-1283) se ejecuta una fila y se avanza el contador:

      ret= MADB_DoExecute(Stmt);

      ++Stmt->ArrayOffset;

MADB_DoExecute acaba en Protocol::executePreparedQuery, que llama a throwStmtError ante cualquier error del servidor. La excepción salta por encima de la llamada a AfterExecute() (línea 1313), que es la que reinicia el contador:

void MADB_Stmt::AfterExecute()
{
  /* All rows processed, so we can unset ArrayOffset */
  ArrayOffset= 0;

Y se captura más arriba, en driver/ma_api_internal.cpp:1145, sin tocar ArrayOffset:

  try
  {
    return Stmt->Methods->Execute(Stmt, false);
  }
  catch (SQLException &e)
  {
    return MADB_FromException(Stmt->Error, e);
  }

Hay además una segunda vía con el mismo efecto: todos los goto end de MADB_StmtExecute se saltan también el reinicio, porque la etiqueta end: (línea 1315) está después de AfterExecute().

El valor viejo se usa en driver/ma_codec.cpp:37:

  IndicatorMapper::IndicatorMapper(MADB_Stmt *Stmt) :
    statusArr(Stmt->Apd->Header.ArrayStatusPtr + Stmt->ArrayOffset),

y se desreferencia en la línea 52:

  bool IndicatorMapper::operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr) {
    if (statusArr && statusArr[row_nr] == SQL_PARAM_IGNORE) {

ArrayStatusPtr es el array que la aplicación pasa con SQL_ATTR_PARAM_OPERATION_PTR, y según la especificación ODBC tiene exactamente SQL_ATTR_PARAMSET_SIZE entradas. Con cualquier desplazamiento distinto de cero, la lectura se sale del array.

La suma del puntero se hace siempre, sin comprobar antes si es NULL, y eso provoca un segundo efecto: si la aplicación no ha configurado SQL_ATTR_PARAM_OPERATION_PTR, ArrayStatusPtr vale NULL; al sumarle el desplazamiento se convierte en un valor pequeño pero distinto de NULL, la comprobación statusArr && deja de protegerlo y se lee una dirección cercana a cero.

Pasos de reproducción

La prueba de concepto (poc/poc_arrayoffset_bulk.c, lanzada con poc/run_poc.sh, incluidas en un zip adjunto al reporte) hace dos lotes de 8 filas sobre el mismo statement:

1. Lote 1: se enlazan los parámetros por filas (row-wise). Así MADB_BulkInsertPossible devuelve falso y el driver usa el bucle fila a fila, que es donde avanza ArrayOffset. La última fila repite una clave primaria que ya existe.
2. El servidor rechaza esa fila, SQLExecute devuelve -1 y la aplicación ve un error normal de clave duplicada.
3. Lote 2: se vuelven a enlazar los parámetros por columnas (column-wise), para que el driver use la ruta de inserción masiva (bulk), y se ejecuta sobre el mismo handle.

Como control negativo, con todas las claves del lote 1 únicas no pasa nada:

[*] batch #1 (row-wise, 8 rows, last row unique -> succeeds)
    SQLExecute rc=0 (ok)
[*] batch #2 (column-wise -> bulk path, 8 rows)
    SQLExecute rc=0
[*] finished without a memory error

Con la clave duplicada en la última fila del lote 1, AddressSanitizer detecta la lectura fuera de límites en el lote 2:

[*] batch #1 (row-wise, 8 rows, last row DUPLICATE PK -> errors)
    SQLExecute rc=-1 (error, as intended)
    [batch1] 23000: Duplicate entry '9999' for key 'PRIMARY'
[*] batch #2 (column-wise -> bulk path, 8 rows)
==43102==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6e0adefe6560
READ of size 2 at 0x6e0adefe6560 thread T0
    #0 mariadb::IndicatorMapper::operator()(...) driver/ma_codec.cpp:52
    #1 withRowCheckCallback driver/cppmini/class/ServerSidePreparedStatement.cpp:381
0x6e0adefe6560 is located 0 bytes after 16-byte region [0x6e0adefe6550,0x6e0adefe6560)

La región de 16 bytes es el array de 8 entradas que la aplicación pasó en SQL_ATTR_PARAM_OPERATION_PTR. Si no se configura ese array, la misma secuencia termina en un fallo de segmentación sobre una dirección casi nula (SEGV on unknown address 0x0e, con rsi = 0x7, que es el desplazamiento que quedó colgado).

Impacto

No hace falta un servidor malicioso ni que la aplicación haga nada raro: basta con un patrón de uso habitual, insertar un lote, que el servidor rechace una fila por una restricción y volver a insertar otro lote sobre el mismo handle. Quien pueda meter un valor en ese lote (por ejemplo, un registro que choque con una clave única existente) decide si se activa el fallo. Por eso el investigador puntúa con PR:L: hace falta ser un usuario aceptado de la aplicación, no un atacante anónimo.

Según el reporte, hay dos consecuencias:

• Denegación de servicio: si la aplicación no usa SQL_ATTR_PARAM_OPERATION_PTR, el proceso cae de forma fiable. En un servicio compartido, eso es una caída provocada con datos de usuario normales.
• Pérdida silenciosa de datos: si el array sí está configurado, el valor leído fuera de límites decide si una fila se trata como SQL_PARAM_IGNORE. Así pueden desaparecer filas de un lote que la aplicación cree insertado, sin ningún error.

El desplazamiento erróneo sobrevive además a cualquier operación posterior con arrays de parámetros en ese handle, no solo a la siguiente, porque nada más lo reinicia.

Al tratarse de una librería cliente sin instancia gestionada por MariaDB, el investigador aclara que no hay confirmación en producción.

Remediación

El reporte figura como resuelto. Las correcciones que propuso el investigador son:

• Reiniciar ArrayOffset en todas las salidas de MADB_StmtExecute, no solo en la que termina bien: con un scope guard en esa función, o moviendo el reinicio al catch de ma_api_internal.cpp:1145, junto a MADB_FromException. Cualquiera de las dos cubre tanto la excepción como los goto end.
• Por separado, que IndicatorMapper compruebe si ArrayStatusPtr es NULL antes de sumarle el desplazamiento, porque ese orden es lo que convierte un NULL en un puntero desbocado.

Qué aprender de este caso

• En código C++ con excepciones, revisa todo estado que se "limpia al final" de una función (contadores, offsets, flags): si el reinicio está en una llamada al final del camino feliz, una excepción o un goto lo esquivan. Un objeto RAII o scope guard que lo reinicie en el destructor cubre todas las salidas.
• Cuando una API traduce excepciones a códigos de error (como el catch que devuelve SQL_ERROR), comprueba que el objeto quede en un estado coherente; si no, el siguiente uso del mismo handle hereda la basura. Probar "falla un lote y luego se reutiliza el handle" es un caso de test que pocos escriben.
• Desconfía de expresiones como ptr + offset seguidas de if (ptr && ...): si ptr puede ser NULL, la suma anula la comprobación. Comprueba NULL sobre el puntero original antes de desplazarlo.
• Para encontrar fallos así en drivers y librerías cliente, combina un error provocado a propósito por el servidor (una clave duplicada basta) con AddressSanitizer y cambios de ruta interna entre ejecuciones (aquí, row-wise frente a column-wise).