Skip to content

Fix boolean buffer overflow #191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
richardm90 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix boolean buffer overflow #191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cabd0b5
test: add BOOLEAN, CHAR, and VARCHAR data type tests
richardm90 Mar 29, 2026
17bf83c
test: add CHAR, VARCHAR, BOOLEAN, and multi-byte UTF-8 data type tests
richardm90 Mar 29, 2026
c4419c3
fix: prevent buffer overread when fetching string column data
richardm90 Mar 29, 2026
e1f91b1
fix: handle BOOLEAN/DATALINK type code differences across IBM i versions
richardm90 Jun 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/db2a.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ const SQL_VARCHAR = 12;
const SQL_BLOB = 13;
const SQL_CLOB = 14;
const SQL_DBCLOB = 15;
const SQL_DATALINK = 16;
const SQL_BOOLEAN = 16;
const SQL_WCHAR = 17;
const SQL_WVARCHAR = 18;
const SQL_BIGINT = 19;
Expand All @@ -362,6 +362,7 @@ const SQL_VARGRAPHIC = 96;
const SQL_LONGVARGRAPHIC = SQL_VARGRAPHIC;
const SQL_BINARY = -2;
const SQL_VARBINARY = -3;
const SQL_DATALINK = -400;
const SQL_LONGVARBINARY = SQL_VARBINARY;
const SQL_DATE = 91;
const SQL_TYPE_DATE = 91;
Expand Down Expand Up @@ -1150,6 +1151,7 @@ module.exports.SQL_VARCHAR = SQL_VARCHAR;
module.exports.SQL_BLOB = SQL_BLOB;
module.exports.SQL_CLOB = SQL_CLOB;
module.exports.SQL_DBCLOB = SQL_DBCLOB;
module.exports.SQL_BOOLEAN = SQL_BOOLEAN;
module.exports.SQL_DATALINK = SQL_DATALINK;
module.exports.SQL_WCHAR = SQL_WCHAR;
module.exports.SQL_WVARCHAR = SQL_WVARCHAR;
Expand Down
22 changes: 21 additions & 1 deletion src/db2ia/dberror.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@
#include <string.h>

#include "sqlcli.h"

// SQL type code 16 is release-dependent at runtime: on IBM i 7.5+ the driver
// describes a BOOLEAN column as 16 and a DATALINK as -400, whereas on 7.4 and
// earlier 16 is DATALINK (there is no BOOLEAN type). The bundled sqlcli.h is
// stale on every release (it defines SQL_DATALINK as 16 and omits SQL_BOOLEAN),
// so the defines below normalise both constants to their 7.5+ runtime values.
//
// Consequence: these constants make code 16 mean BOOLEAN. On 7.4 a DATALINK
// column (described as 16) is therefore routed through the BOOLEAN path. This
// is intentional -- DATALINK is effectively unused and is not supported on 7.4.
#ifndef SQL_BOOLEAN
#define SQL_BOOLEAN 16
#endif

#ifdef SQL_DATALINK
#undef SQL_DATALINK
#endif
#define SQL_DATALINK -400
#include "napi.h"

#define DEBUG(object, f_, ...) \
Expand Down Expand Up @@ -133,7 +151,9 @@ static const char* getSQLType(int sqlType)
return "CLOB";
case SQL_DBCLOB: // SQL_DBCLOB = 15
return "DBCLOB";
case SQL_DATALINK: // SQL_DATALINK = 16
case SQL_BOOLEAN: // SQL_BOOLEAN = 16 (7.5+)
return "BOOLEAN";
case SQL_DATALINK: // SQL_DATALINK = -400 (7.5+), was 16 (pre-7.5)
return "DATALINK";
case SQL_WCHAR: // SQL_WCHAR = 17
return "WCHAR";
Expand Down
74 changes: 50 additions & 24 deletions src/db2ia/dbstmt.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ Napi::Object DbStmt::Init(Napi::Env env, Napi::Object exports)
* function takes 0 or 1 argument.
* info[0] (Boolean): true for ON false for OFF.
* Returns: boolean true/false indicating the state of the debug switch.
*
*
*/
Napi::Value DbStmt::AsNumber(const Napi::CallbackInfo &info)
{
Expand Down Expand Up @@ -186,15 +186,15 @@ Napi::Value DbStmt::SetStmtAttr(const Napi::CallbackInfo &info)
/*
* DbStmt::GetStmtAttr
* Description: Returns the current settings for the specified statement option
* Parameters:
* Parameters:
* const Napi::CallbackInfo& info:
* The information passed by Napi from the JavaScript call, including
* arguments from the JavaScript function. In JavaScript, the exported
* function takes two arguments, stored on the info object:
* info[0] (Number): Attribute is the statement attribute to set.
* Refer to the attribute table for more details.
* Return: The attribute option in the format of a Number or a String.
*
*
*/
Napi::Value DbStmt::GetStmtAttr(const Napi::CallbackInfo &info)
{
Expand Down Expand Up @@ -275,7 +275,7 @@ class ExecAsyncWorker : public Napi::AsyncWorker
// - SQL_ERROR
// - SQL_INVALID_HANDLE
// - SQL_NO_DATA_FOUND
// SQL_NO_DATA_FOUND is returned if the SQL statement is a Searched UPDATE
// SQL_NO_DATA_FOUND is returned if the SQL statement is a Searched UPDATE
// or Searched DELETE and no rows satisfy the search condition.
if (sqlReturnCode == SQL_SUCCESS_WITH_INFO)
{
Expand Down Expand Up @@ -744,7 +744,7 @@ class BindParamAsyncWorker : public Napi::AsyncWorker
* Parameters:
* const Napi::CallbackInfo& info:
* The information passed by Napi from the JavaScript call. Contains 2 parameters,
*
*
* info[0]: [Array]: An array of arrays, the inner arrays containing
* the data to bind to the prepared statement.
* info[1]: [Function]: The callback function, with
Expand Down Expand Up @@ -896,7 +896,7 @@ class BindParametersAsyncWorker : public Napi::AsyncWorker
* Parameters:
* const Napi::CallbackInfo& info:
* The information passed by Napi from the JavaScript call. Contains 2 parameters,
*
*
* info[0]: [Array]: An array of the data to bind to the prepared statement.
* info[1]: [Function]: The callback function, with
* arguments passed to it in the format function(error):
Expand Down Expand Up @@ -1134,7 +1134,7 @@ void DbStmt::Execute(const Napi::CallbackInfo &info)

/*
* DbStmt::ExecuteSync
* Syntex: executeSync(), executeSync(function(OutputParameters, error))
* Syntex: executeSync(), executeSync(function(OutputParameters, error))
* Description:
* Runs the "Execute" workflow synchronously, blocking the Node.js event
* loop. Takes a statement prepared with "Prepare" and possibly bound
Expand Down Expand Up @@ -1366,7 +1366,7 @@ class FetchAsyncWorker : public Napi::AsyncWorker
* Syntex 1: fetch(function Callback(Row, ReturnCode/error))
* Syntex 2: fetch(int Orient, int Offset, function Callback(Row, ReturnCode/error))
* Description:
* Advances the cursor to the next row of the result set, and retrieves any bound columns.
* Advances the cursor to the next row of the result set, and retrieves any bound columns.
* Or positions the cursor based on the requested orientation.
* Parameters:
* const Napi::CallbackInfo& info:
Expand Down Expand Up @@ -2347,9 +2347,14 @@ int DbStmt::bindColData(Napi::Env env)
sqlReturnCode = SQLBindCol(stmth, col + 1, SQL_C_CHAR, (SQLPOINTER)bindingRowInC[col], maxColLen, &dbColumn[col].rlength);
}
break;
default: // SQL_CHAR / SQL_VARCHAR
default: // SQL_CHAR / SQL_VARCHAR / SQL_BOOLEAN and other string-representable types

@kadler kadler Apr 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I think we should bind booleans as a boolean type not as a string anyway. As mentioned in the PR description, the value of SQL_DATALINK was changed in 7.5 so that SQL_BOOLEAN could use its value for consistency with Db2 LUW. I argued against this change, but it was determined that there would be minimal impact because basically nobody uses DATALINKs anyway so we may as well do the same.

  • add code define SQL_BOOLEAN (and redefine SQL_DATALINK) if not defined
  • adjust db2a.js to set SQL_DATALINK to -400.
  • add SQL_BOOLEAN to db2a.js with value 16

@richardm90 richardm90 May 11, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kadler, apologies for the delay, I'd turned off notifications some time ago and had forgotten that fact when I raised this PR. I'll have a look at the changes you suggest.

@richardm90 richardm90 Jun 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kadler, I've just implemented the changes to correctly handle the boolean data type.

The SQL type code 16 has different meanings across IBM i releases:

  • V7R4 and earlier: type 16 = DATALINK (no BOOLEAN type exists)
  • V7R5 and later: type 16 = BOOLEAN, type -400 = DATALINK

Additionally, boolean parameter binding in master was broken - it used
SQL_C_BIT which IBM i CLI rejects with error HY003.

I've made three key changes in this PR:

  1. Normalize Type Constants (dberror.h)
  2. Fix Boolean Parameter Binding (dbstmt.cc)
  3. Improve String Column Handling (dbstmt.cc)

Backward Compatibility

✅ On V7R4: DATALINK columns (type 16) correctly returned as strings
✅ On V7R5+: BOOLEAN columns (type 16) correctly returned as booleans
✅ On V7R5+: DATALINK columns (type -400) correctly returned as strings

When running the tests on V7R4 (or lower) there will be four failures, the new boolean tests; the new datalink test will succeed. When running the tests on V7R5 (or higher) all test are successful.

Note that the datalink data type is not well supported by the connector. I have added tests that check for a datalink column in the result but work would be required to handle parameter binding.

I've not added any comments to README.md about changes to the BOOLEAN data type handling. Anybody already using the connector and BOOLEAN will now be dealing with a true boolean rather than a string representation. Happy to do that if you think it's appropriate.

{
// colPrecise * 4 + 1 accounts for multi-byte character expansion + null terminator.
// Minimum of 6 ensures types like BOOLEAN (colPrecise=1, but string representation
// "FALSE" needs 6 bytes) have a large enough buffer.
maxColLen = dbColumn[col].colPrecise * 4 + 1;
if (maxColLen < 6)
maxColLen = 6;
bindingRowInC[col] = (SQLCHAR *)calloc(maxColLen, sizeof(SQLCHAR));
sqlReturnCode = SQLBindCol(stmth, col + 1, SQL_C_CHAR, (SQLPOINTER)bindingRowInC[col], maxColLen, &dbColumn[col].rlength);
}
Expand Down Expand Up @@ -2430,8 +2435,12 @@ int DbStmt::fetchData()
}
else
{
// rlength is the actual data length (e.g. 5 for CHAR(5) or "FALSE").
// Allocate colLen + 1 to ensure null termination, since memcpy copies
// only the data bytes without a null terminator. The extra byte is
// zero-filled by calloc.
colLen = dbColumn[col].rlength;
rowOfResultSetInC[col].data = (SQLCHAR *)calloc(colLen, sizeof(SQLCHAR));
rowOfResultSetInC[col].data = (SQLCHAR *)calloc(colLen + 1, sizeof(SQLCHAR));
memcpy(rowOfResultSetInC[col].data, bindingRowInC[col], colLen * sizeof(SQLCHAR));
rowOfResultSetInC[col].rlength = colLen;
}
Expand Down Expand Up @@ -2467,6 +2476,16 @@ int DbStmt::buildJsObject(Napi::Env env, Napi::Array *array)
});
break;
}
case SQL_BOOLEAN:
{
// BOOLEAN comes back as the null-terminated string "TRUE" or "FALSE".
// The CLI reports the length indicator as SQL_NTS rather than an
// explicit byte count, so compare the string value instead of relying
// on rlength. NULL is already handled before this switch (JS null).
bool boolValue = (strcmp((const char *)resultSetInC[row][col].data, "TRUE") == 0);
value = Napi::Boolean::New(env, boolValue);
break;
}
case SQL_SMALLINT: // -32768 to +32767
case SQL_INTEGER: // -2147483648 to +2147483647
// case SQL_BIGINT: // -9223372036854775808 to +9223372036854775807
Expand All @@ -2489,7 +2508,12 @@ int DbStmt::buildJsObject(Napi::Env env, Napi::Array *array)
break;
}
default:
value = Napi::String::New(env, resultSetInC[row][col].data);
// Use the known data length to bound string creation rather than relying
// on null termination, as a safeguard against buffer overreads.
if (resultSetInC[row][col].rlength == SQL_NTS)
value = Napi::String::New(env, (const char *)resultSetInC[row][col].data);
else
value = Napi::String::New(env, (const char *)resultSetInC[row][col].data, resultSetInC[row][col].rlength);
break;
}
}
Expand Down Expand Up @@ -2587,7 +2611,7 @@ int DbStmt::bindParams(Napi::Env env, Napi::Array *params, std::string &error)
error = "BIND INDICATOR TYPE OF PARAMETER " + std::to_string(i + 1) + " IS INVALID\n";
return -1;
}

param[i].io = io;
bindIndicator = bindValue.ToNumber().Int32Value(); //convert from Napi::Value to an int

Expand Down Expand Up @@ -2644,11 +2668,12 @@ int DbStmt::bindParams(Napi::Env env, Napi::Array *params, std::string &error)
}
else if (bindIndicator == 5 || value.IsBoolean())
{ //Parameter is Boolean
bool *boolean = (bool *)malloc(sizeof(bool));
*boolean = value.ToBoolean();
param[i].valueType = SQL_C_BIT;
param[i].buf = boolean;
param[i].ind = 0;
// The IBM i CLI rejects SQL_C_BIT for a BOOLEAN parameter (HY003), so
// bind the value as the character string "TRUE"/"FALSE", which Db2
// casts to BOOLEAN. strdup allocates exactly strlen+1 (freed in freeSp).
param[i].valueType = SQL_C_CHAR;
param[i].buf = strdup(value.ToBoolean() ? "TRUE" : "FALSE");
param[i].ind = SQL_NTS;
}
else if (bindIndicator == SQL_BINARY || bindIndicator == SQL_BLOB || value.IsBuffer())
{ //Parameter is blob/binary
Expand Down Expand Up @@ -2682,11 +2707,12 @@ int DbStmt::bindParams(Napi::Env env, Napi::Array *params, std::string &error)
}
else if (value.IsBoolean())
{ //Parameter is Boolean
bool *boolean = (bool *)malloc(sizeof(bool));
*boolean = value.ToBoolean();
param[i].valueType = SQL_C_BIT;
param[i].buf = boolean;
param[i].ind = 0;
// The IBM i CLI rejects SQL_C_BIT for a BOOLEAN parameter (HY003), so
// bind the value as the character string "TRUE"/"FALSE", which Db2
// casts to BOOLEAN. strdup allocates exactly strlen+1 (freed in freeSp).
param[i].valueType = SQL_C_CHAR;
param[i].buf = strdup(value.ToBoolean() ? "TRUE" : "FALSE");
param[i].ind = SQL_NTS;
}
else switch(param[i].paramType)
{
Expand Down Expand Up @@ -2752,7 +2778,7 @@ int DbStmt::bindParams(Napi::Env env, Napi::Array *params, std::string &error)
{
std::string string = value.ToString().Utf8Value();
const char *cString = string.c_str();
if(strlen(cString) > 0)
if(strlen(cString) > 0)
{
strcpy((char *)param[i].buf, cString);
param[i].ind = strlen(cString);
Expand Down Expand Up @@ -2792,7 +2818,7 @@ int DbStmt::bindParams(Napi::Env env, Napi::Array *params, std::string &error)
break;
}
}

//link to doc https://www.ibm.com/support/knowledgecenter/en/ssw_ibm_i_73/cli/rzadpfnbndpm.htm
sqlReturnCode = SQLBindParameter(
stmth, //SQLHSTMT statement handle
Expand Down
Loading