Skip to content

lib/gis: Fix security vulnerabilities and reliability bugs in core modules#7596

Draft
NeelGhoshal wants to merge 10 commits into
OSGeo:mainfrom
NeelGhoshal:fix/lib-gis-security-reliability-medium-coverage
Draft

lib/gis: Fix security vulnerabilities and reliability bugs in core modules#7596
NeelGhoshal wants to merge 10 commits into
OSGeo:mainfrom
NeelGhoshal:fix/lib-gis-security-reliability-medium-coverage

Conversation

@NeelGhoshal

Copy link
Copy Markdown
Contributor

Description

Fix a collection of security, reliability, and maintainability issues
across several core lib/gis modules, identified by static analysis.

Changes by file:

File Category Fix
mapset_nme.c Security Add field-width specifier to %s format placeholder to prevent buffer overread
open.c Security Eliminate TOCTOU race condition window when opening files
error.c Reliability Guard prefix_std array access against negative/overflowing index (×2)
error.c Maintainability Replace non-reentrant ctime() with ctime_r()
mapset_msc.c Reliability Fix path access at negative byte offset -1
whoami.c Maintainability Replace non-reentrant getpwuid() with getpwuid_r()
adj_cellhd.c Maintainability Reduce cognitive complexity of three functions (55→25, 48→25, 30→25)

Motivation and context

These issues were flagged by static analysis (SonarQube). The security
fixes address real vulnerabilities (format string, TOCTOU). The reliability
fixes prevent undefined behaviour from out-of-bounds array access. The
maintainability changes improve thread safety and readability.

How has this been tested?

The changed modules are exercised indirectly through raster operations,
mapset management tools, and integration tests throughout the test suite.
No dedicated unit tests exist for these specific functions.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • PR title provides summary of the changes and starts with one of the pre-defined prefixes
  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have added tests to cover my changes

@github-actions github-actions Bot added C Related code is in C libraries labels Jun 23, 2026
@echoix echoix marked this pull request as draft June 23, 2026 17:30
@echoix

echoix commented Jun 23, 2026

Copy link
Copy Markdown
Member

Make sure it compiles before we take a look

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

pre-commit

[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 575 to 661 in 6821a46

/* put everything through ll_format + ll_scan */
G_llres_format(cellhd->ns_res, buf);
if (G_llres_scan(buf, &new) != 1)
G_fatal_error(_("Invalid NS resolution"));
cellhd->ns_res = new;
G_llres_format(cellhd->ew_res, buf);
if (G_llres_scan(buf, &new) != 1)
G_fatal_error(_("Invalid EW resolution"));
cellhd->ew_res = new;
G_lat_format(cellhd->north, buf);
if (G_lat_scan(buf, &new) != 1)
G_fatal_error(_("Invalid North"));
cellhd->north = new;
G_lat_format(cellhd->south, buf);
if (G_lat_scan(buf, &new) != 1)
G_fatal_error(_("Invalid South"));
cellhd->south = new;
G_lon_format(cellhd->west, buf);
if (G_lon_scan(buf, &new) != 1)
G_fatal_error(_("Invalid West"));
cellhd->west = new;
G_lon_format(cellhd->east, buf);
if (G_lon_scan(buf, &new) != 1)
G_fatal_error(_("Invalid East"));
cellhd->east = new;
/* convert to seconds */
cellhds = *cellhd;
old = cellhds.ns_res * 3600;
snprintf(buf, sizeof(buf), "%f", old);
sscanf(buf, "%lf", &new);
cellhds.ns_res = new;
old = cellhds.ew_res * 3600;
snprintf(buf, sizeof(buf), "%f", old);
sscanf(buf, "%lf", &new);
cellhds.ew_res = new;
old = cellhds.north * 3600;
snprintf(buf, sizeof(buf), "%f", old);
sscanf(buf, "%lf", &new);
cellhds.north = new;
old = cellhds.south * 3600;
snprintf(buf, sizeof(buf), "%f", old);
sscanf(buf, "%lf", &new);
cellhds.south = new;
old = cellhds.west * 3600;
snprintf(buf, sizeof(buf), "%f", old);
sscanf(buf, "%lf", &new);
cellhds.west = new;
old = cellhds.east * 3600;
snprintf(buf, sizeof(buf), "%f", old);
sscanf(buf, "%lf", &new);
cellhds.east = new;
ll_adjust = 0;
/* N - S */
/* resolution */
res_adj = 0;
old = cellhds.ns_res;
if (old > 0.4) {
/* round to nearest 0.1 sec */
dsec = old * 10;
dsec2 = floor(dsec + 0.5);
new = dsec2 / 10;
diff = fabs(dsec2 - dsec) / dsec;
if (diff > 0 && diff < llepsilon) {
G_llres_format(old / 3600, buf);
G_llres_format(new / 3600, buf2);
if (strcmp(buf, buf2))
G_verbose_message(_("NS resolution rounded from %s to %s"), buf,
buf2);
ll_adjust = 1;
res_adj = 1;
cellhds.ns_res = new;
}


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 664 to 665 in 6821a46

if (res_adj) {
double n_off, s_off;


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 667 to 671 in 6821a46

old = cellhds.north;
dsec = old * 10;
dsec2 = floor(dsec + 0.5);
diff = fabs(dsec2 - dsec) / (cellhds.ns_res * 10);
n_off = diff;


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 673 to 677 in 6821a46

old = cellhds.south;
dsec = old * 10;
dsec2 = floor(dsec + 0.5);
diff = fabs(dsec2 - dsec) / (cellhds.ns_res * 10);
s_off = diff;


[pre-commit] reported by reviewdog 🐶

if (n_off < llepsilon || n_off <= s_off) {


[pre-commit] reported by reviewdog 🐶

diff = n_off;


[pre-commit] reported by reviewdog 🐶

cellhds.north = new;


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 694 to 706 in 6821a46

old = cellhds.south;
new = cellhds.north - cellhds.ns_res *cellhds.rows;
diff = fabs(new - old) / cellhds.ns_res;
if (diff > 0) {
G_lat_format(old / 3600, buf);
G_lat_format(new / 3600, buf2);
if (strcmp(buf, buf2))
G_verbose_message(_("South adjusted from %s to %s"), buf,
buf2);
}
cellhds.south = new;
}
else {


[pre-commit] reported by reviewdog 🐶

diff = s_off;


[pre-commit] reported by reviewdog 🐶

cellhds.south = new;


[pre-commit] reported by reviewdog 🐶


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 721 to 726 in 6821a46

old = cellhds.north;
new = cellhds.south + cellhds.ns_res *cellhds.rows;
diff = fabs(new - old) / cellhds.ns_res;
if (diff > 0) {
G_lat_format(old / 3600, buf);
G_lat_format(new / 3600, buf2);


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 728 to 729 in 6821a46

G_verbose_message(_("North adjusted from %s to %s"), buf,
buf2);


[pre-commit] reported by reviewdog 🐶

cellhds.north = new;


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 733 to 786 in 6821a46

}
else {
old = cellhds.north;
dsec = old * 10;
dsec2 = floor(dsec + 0.5);
new = dsec2 / 10;
diff = fabs(dsec2 - dsec) / (cellhds.ns_res * 10);
if (diff > 0 && diff < llepsilon) {
G_lat_format(old / 3600, buf);
G_lat_format(new / 3600, buf2);
if (strcmp(buf, buf2))
G_verbose_message(_("North rounded from %s to %s"), buf, buf2);
ll_adjust = 1;
cellhds.north = new;
}
old = cellhds.south;
dsec = old * 10;
dsec2 = floor(dsec + 0.5);
new = dsec2 / 10;
diff = fabs(dsec2 - dsec) / (cellhds.ns_res * 10);
if (diff > 0 && diff < llepsilon) {
G_lat_format(old / 3600, buf);
G_lat_format(new / 3600, buf2);
if (strcmp(buf, buf2))
G_verbose_message(_("South rounded from %s to %s"), buf, buf2);
ll_adjust = 1;
cellhds.south = new;
}
}
cellhds.ns_res = (cellhds.north - cellhds.south) / cellhds.rows;
/* E - W */
/* resolution */
res_adj = 0;
old = cellhds.ew_res;
if (old > 0.4) {
/* round to nearest 0.1 sec */
dsec = old * 10;
dsec2 = floor(dsec + 0.5);
new = dsec2 / 10;
diff = fabs(dsec2 - dsec) / dsec;
if (diff > 0 && diff < llepsilon) {
G_llres_format(old / 3600, buf);
G_llres_format(new / 3600, buf2);
if (strcmp(buf, buf2))
G_verbose_message(_("EW resolution rounded from %s to %s"), buf,
buf2);
ll_adjust = 1;
res_adj = 1;
cellhds.ew_res = new;
}
}


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 788 to 789 in 6821a46

if (res_adj) {
double w_off, e_off;


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 791 to 801 in 6821a46

old = cellhds.west;
dsec = old * 10;
dsec2 = floor(dsec + 0.5);
diff = fabs(dsec2 - dsec) / (cellhds.ew_res * 10);
w_off = diff;
old = cellhds.east;
dsec = old * 10;
dsec2 = floor(dsec + 0.5);
diff = fabs(dsec2 - dsec) / (cellhds.ew_res * 10);
e_off = diff;


[pre-commit] reported by reviewdog 🐶

if (w_off < llepsilon || w_off <= e_off) {


[pre-commit] reported by reviewdog 🐶

diff = w_off;


[pre-commit] reported by reviewdog 🐶

cellhds.west = new;


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 818 to 830 in 6821a46

old = cellhds.east;
new = cellhds.west + cellhds.ew_res *cellhds.cols;
diff = fabs(new - old) / cellhds.ew_res;
if (diff > 0) {
G_lon_format(old / 3600, buf);
G_lon_format(new / 3600, buf2);
if (strcmp(buf, buf2))
G_verbose_message(_("East adjusted from %s to %s"), buf,
buf2);
}
cellhds.east = new;
}
else {


[pre-commit] reported by reviewdog 🐶

diff = e_off;


[pre-commit] reported by reviewdog 🐶

cellhds.east = new;


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 844 to 870 in 6821a46

old = cellhds.west;
new = cellhds.east - cellhds.ew_res *cellhds.cols;
diff = fabs(new - cellhds.west) / cellhds.ew_res;
if (diff > 0) {
G_lon_format(old / 3600, buf);
G_lon_format(new / 3600, buf2);
if (strcmp(buf, buf2))
G_verbose_message(_("West adjusted from %s to %s"), buf,
buf2);
}
cellhds.west = new;
}
}
else {
old = cellhds.west;
dsec = old * 10;
dsec2 = floor(dsec + 0.5);
new = dsec2 / 10;
diff = fabs(dsec2 - dsec) / (cellhds.ew_res * 10);
if (diff > 0 && diff < llepsilon) {
G_lon_format(old / 3600, buf);
G_lon_format(new / 3600, buf2);
if (strcmp(buf, buf2))
G_verbose_message(_("West rounded from %s to %s"), buf, buf2);
ll_adjust = 1;
cellhds.west = new;


[pre-commit] reported by reviewdog 🐶


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 873 to 894 in 6821a46

old = cellhds.east;
dsec = old * 10;
dsec2 = floor(dsec + 0.5);
new = dsec2 / 10;
diff = fabs(dsec2 - dsec) / (cellhds.ew_res * 10);
if (diff > 0 && diff < llepsilon) {
G_lon_format(old / 3600, buf);
G_lon_format(new / 3600, buf2);
if (strcmp(buf, buf2))
G_verbose_message(_("East rounded from %s to %s"), buf, buf2);
ll_adjust = 1;
cellhds.east = new;
}
}
cellhds.ew_res = (cellhds.east - cellhds.west) / cellhds.cols;
cellhd->ns_res = cellhds.ns_res / 3600;
cellhd->ew_res = cellhds.ew_res / 3600;
cellhd->north = cellhds.north / 3600;
cellhd->south = cellhds.south / 3600;
cellhd->west = cellhds.west / 3600;
cellhd->east = cellhds.east / 3600;


[pre-commit] reported by reviewdog 🐶

grass/lib/gis/adj_cellhd.c

Lines 896 to 897 in 6821a46

return ll_adjust;
}

Comment thread lib/gis/adj_cellhd.c
Comment on lines +33 to +34
static void check_ns_ew_3d_input(const struct Cell_head *cellhd,
int row_flag, int col_flag);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
static void check_ns_ew_3d_input(const struct Cell_head *cellhd,
int row_flag, int col_flag);
static void check_ns_ew_3d_input(const struct Cell_head *cellhd, int row_flag,
int col_flag);

Comment thread lib/gis/adj_cellhd.c
Comment on lines +65 to +66
check_ns_input(cellhd, row_flag);
check_ew_input(cellhd, col_flag);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
check_ns_input(cellhd, row_flag);
check_ew_input(cellhd, col_flag);
check_ns_input(cellhd, row_flag);
check_ew_input(cellhd, col_flag);

Comment thread lib/gis/adj_cellhd.c
{
if (!row_flag) {
if (cellhd->ns_res <= 0)
G_fatal_error(_("Illegal n-s resolution value: %g"), cellhd->ns_res);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
G_fatal_error(_("Illegal n-s resolution value: %g"), cellhd->ns_res);
G_fatal_error(_("Illegal n-s resolution value: %g"),
cellhd->ns_res);

Comment thread lib/gis/adj_cellhd.c
{
if (!col_flag) {
if (cellhd->ew_res <= 0)
G_fatal_error(_("Illegal e-w resolution value: %g"), cellhd->ew_res);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
G_fatal_error(_("Illegal e-w resolution value: %g"), cellhd->ew_res);
G_fatal_error(_("Illegal e-w resolution value: %g"),
cellhd->ew_res);

Comment thread lib/gis/adj_cellhd.c
if (cellhd->depths <= 0)
G_fatal_error(_("Illegal depths value: %d"), cellhd->depths);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change

Comment thread lib/gis/whoami.c
Comment on lines +52 to +54
if (!name || !*name) {
struct passwd pwd;
struct passwd *result = NULL;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
if (!name || !*name) {
struct passwd pwd;
struct passwd *result = NULL;
if (!name || !*name) {
struct passwd pwd;
struct passwd *result = NULL;

Comment thread lib/gis/whoami.c
Comment on lines +56 to +58
long buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
if (buflen < 0)
buflen = 16384; /* fallback */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
long buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
if (buflen < 0)
buflen = 16384; /* fallback */
long buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
if (buflen < 0)
buflen = 16384; /* fallback */

Comment thread lib/gis/whoami.c
if (buflen < 0)
buflen = 16384; /* fallback */

char *buf = G_malloc((size_t)buflen);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
char *buf = G_malloc((size_t)buflen);
char *buf = G_malloc((size_t)buflen);

Comment thread lib/gis/whoami.c
Comment on lines +62 to 65
if (getpwuid_r(getuid(), &pwd, buf, (size_t)buflen, &result) == 0 &&
result && result->pw_name && *result->pw_name) {
name = G_store(result->pw_name);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
if (getpwuid_r(getuid(), &pwd, buf, (size_t)buflen, &result) == 0 &&
result && result->pw_name && *result->pw_name) {
name = G_store(result->pw_name);
}
if (getpwuid_r(getuid(), &pwd, buf, (size_t)buflen, &result) == 0 &&
result && result->pw_name && *result->pw_name) {
name = G_store(result->pw_name);
}

Comment thread lib/gis/whoami.c
Comment on lines +67 to +68
G_free(buf);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[pre-commit] reported by reviewdog 🐶

Suggested change
G_free(buf);
}
G_free(buf);
}

@NeelGhoshal

Copy link
Copy Markdown
Contributor Author

Make sure it compiles before we take a look

Apologies for the inconvenience, my workflow didn't work as I expected it to. I'm working on fixing the issues in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C Related code is in C libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants