diff --git a/db/db.connect/main.c b/db/db.connect/main.c index a6d6bfdeb49..613c78ea83c 100644 --- a/db/db.connect/main.c +++ b/db/db.connect/main.c @@ -328,32 +328,54 @@ int main(int argc, char *argv[]) char *substitute_variables(dbConnection *conn) { char *database, *c, buf[GPATH_MAX]; + int ret; if (!conn->databaseName) return NULL; database = (char *)G_malloc(GPATH_MAX); - strcpy(database, conn->databaseName); + ret = snprintf(database, GPATH_MAX, "%s", conn->databaseName); + if (ret >= GPATH_MAX) { + G_fatal_error(_("Database name too long (exceeds %d characters): %s"), + GPATH_MAX - 1, conn->databaseName); + } - strcpy(buf, database); + snprintf(buf, GPATH_MAX, "%s", database); c = (char *)strstr(buf, "$GISDBASE"); if (c != NULL) { *c = '\0'; - snprintf(database, GPATH_MAX, "%s%s%s", buf, G_gisdbase(), c + 9); + ret = snprintf(database, GPATH_MAX, "%s%s%s", buf, G_gisdbase(), c + 9); + if (ret >= GPATH_MAX) { + G_fatal_error(_("Database path after $GISDBASE expansion too long " + "(exceeds %d characters)"), + GPATH_MAX - 1); + } } - strcpy(buf, database); + snprintf(buf, GPATH_MAX, "%s", database); c = (char *)strstr(buf, "$LOCATION_NAME"); if (c != NULL) { *c = '\0'; - snprintf(database, GPATH_MAX, "%s%s%s", buf, G_location(), c + 14); + ret = + snprintf(database, GPATH_MAX, "%s%s%s", buf, G_location(), c + 14); + if (ret >= GPATH_MAX) { + G_fatal_error( + _("Database path after $LOCATION_NAME expansion too long " + "(exceeds %d characters)"), + GPATH_MAX - 1); + } } - strcpy(buf, database); + snprintf(buf, GPATH_MAX, "%s", database); c = (char *)strstr(buf, "$MAPSET"); if (c != NULL) { *c = '\0'; - snprintf(database, GPATH_MAX, "%s%s%s", buf, G_mapset(), c + 7); + ret = snprintf(database, GPATH_MAX, "%s%s%s", buf, G_mapset(), c + 7); + if (ret >= GPATH_MAX) { + G_fatal_error(_("Database path after $MAPSET expansion too long " + "(exceeds %d characters)"), + GPATH_MAX - 1); + } } #ifdef __MINGW32__ if (strcmp(conn->driverName, "sqlite") == 0 || diff --git a/db/db.connect/tests/conftest.py b/db/db.connect/tests/conftest.py new file mode 100644 index 00000000000..1aaabd1de29 --- /dev/null +++ b/db/db.connect/tests/conftest.py @@ -0,0 +1,15 @@ +import os + +import pytest + +import grass.script as gs + + +@pytest.fixture(scope="module") +def session(tmp_path_factory): + """Set up a GRASS session for db.connect tests.""" + tmp_path = tmp_path_factory.mktemp("grass_session") + project = tmp_path / "test_project" + gs.create_project(project) + with gs.setup.init(project, env=os.environ.copy()) as session: + yield session diff --git a/db/db.connect/tests/db_connect_buffer_overflow_test.py b/db/db.connect/tests/db_connect_buffer_overflow_test.py new file mode 100644 index 00000000000..2b6e4dab7f9 --- /dev/null +++ b/db/db.connect/tests/db_connect_buffer_overflow_test.py @@ -0,0 +1,80 @@ +import pytest + +import grass.script as gs +from grass.exceptions import CalledModuleError +from grass.tools import Tools + + +def test_db_connect_normal_database_name(session): + """Test that db.connect works with normal database names.""" + tools = Tools(session=session) + tools.db_connect(flags="d") + output = tools.db_connect(flags="p", format="json") + assert output["driver"] == "sqlite" + assert "sqlite.db" in output["database"] + + +def test_db_connect_long_valid_database_name(session): + """Test that db.connect works with long but valid database names.""" + long_name = "/tmp/" + "a" * 2000 + "/test.db" + + tools = Tools(session=session) + try: + tools.db_connect(driver="sqlite", database=long_name) + output = gs.read_command("db.connect", flags="p", env=session.env) + assert "test.db" in output + except CalledModuleError as e: + error_msg = str(e).lower() + assert "too long" not in error_msg + assert "exceeds" not in error_msg + + +def test_db_connect_overlong_database_name_rejected(session): + """Test that printing a stored database name exceeding GPATH_MAX fails. + + Setting the connection accepts any string without validation. The overflow + check fires in substitute_variables(), which is called only in print mode. + """ + overlong_name = "/tmp/" + "x" * 4500 + "/test.db" + + tools = Tools(session=session) + tools.db_connect(driver="sqlite", database=overlong_name) + with pytest.raises(CalledModuleError, match=r"too long|exceeds.*characters"): + tools.db_connect(flags="p") + + +def test_db_connect_variable_expansion_overflow(session): + """Test that variable expansion is bounds-checked.""" + gisenv = gs.gisenv(env=session.env) + gisdbase = gisenv["GISDBASE"] + location = gisenv["LOCATION_NAME"] + mapset = gisenv["MAPSET"] + + expanded_length = len(gisdbase) + len(location) + len(mapset) + 100 + padding_needed = 4096 - expanded_length + 500 + + if padding_needed > 0: + overlong_template = ( + "$GISDBASE/" + "x" * padding_needed + "/$LOCATION_NAME/$MAPSET/test.db" + ) + + # Setting the connection stores the template without expanding it. + # The overflow check runs in substitute_variables(), called only when + # printing (-p), where variables are expanded and the buffer overflows. + tools = Tools(session=session) + tools.db_connect(driver="sqlite", database=overlong_template) + with pytest.raises(CalledModuleError, match=r"too long|exceeds.*characters"): + tools.db_connect(flags="p") + + +def test_db_connect_variable_expansion_normal(session): + """Test that normal variable expansion works correctly.""" + template = "$GISDBASE/$LOCATION_NAME/$MAPSET/sqlite/test.db" + + tools = Tools(session=session) + tools.db_connect(driver="sqlite", database=template) + output = tools.db_connect(flags="p", format="json") + assert output["database_template"] == template + assert "$GISDBASE" not in output["database"] + assert "$LOCATION_NAME" not in output["database"] + assert "$MAPSET" not in output["database"]