From c96232cae49b33670cb8d20af955d929c81bf42b Mon Sep 17 00:00:00 2001 From: Esteban82 Date: Fri, 16 Oct 2026 15:16:05 -0300 Subject: [PATCH 1/4] Fix segfault, lost sign and multi-record overrun for numeric -D in mgd77magref parse() stored the Dst value in a buffer that was only allocated later, and skipped the leading minus sign when converting. With several input records a numeric -D also read past the one-element dst array. Co-Authored-By: Claude Sonnet 5.5 --- src/mgd77/cm4_functions.c | 2 +- src/mgd77/mgd77magref.c | 8 ++++---- test/mgd77/magref_dst.sh | 19 +++++++++++++++++++ 3 files changed, 24 insertions(+), 5 deletions(-) create mode 100755 test/mgd77/magref_dst.sh diff --git a/src/mgd77/cm4_functions.c b/src/mgd77/cm4_functions.c index edcc135af81..7896e2c0b10 100644 --- a/src/mgd77/cm4_functions.c +++ b/src/mgd77/cm4_functions.c @@ -459,7 +459,7 @@ int MGD77_cm4field (struct GMT_CTRL *GMT, struct MGD77_CM4 *Ctrl, double *p_lon, /* See if we are using a constant time or an array */ if (Ctrl->CM4_DATA.n_times > 1) { date = p_date[n]; - dst = Ctrl->CM4_D.dst[n]; + dst = Ctrl->CM4_D.index ? Ctrl->CM4_D.dst[n] : Ctrl->CM4_D.dst[0]; mut_now = mut[n]; } else { diff --git a/src/mgd77/mgd77magref.c b/src/mgd77/mgd77magref.c index e53fdd3896c..25f706413c8 100644 --- a/src/mgd77/mgd77magref.c +++ b/src/mgd77/mgd77magref.c @@ -237,10 +237,10 @@ static int parse (struct GMT_CTRL *GMT, struct MGD77MAGREF_CTRL *Ctrl, struct GM break; case 'D': n_errors += gmt_M_repeated_module_option (API, Ctrl->D.active); - j = 0; - if (opt->arg[j] == '-') j++; + j = (opt->arg[0] == '-') ? 1 : 0; if ((opt->arg[j] > 47) && (opt->arg[j] < 58)) { /* arg is numeric -> Dst Index */ - Ctrl->CM4->CM4_D.dst[0] = atof (&opt->arg[j]); + if (Ctrl->CM4->CM4_D.dst == NULL) Ctrl->CM4->CM4_D.dst = calloc (1U, sizeof(double)); + Ctrl->CM4->CM4_D.dst[0] = atof (opt->arg); Ctrl->CM4->CM4_D.index = false; } else { @@ -494,7 +494,7 @@ EXTERN_MSC int GMT_mgd77magref (void *V_API, int mode, void *args) { if (Ctrl->A.fixed_alt) t_col = 2; /* Since we are missing the altitude column */ - Ctrl->CM4->CM4_D.dst = calloc (1U, sizeof(double)); /* We need at least a size of one in case a value is given in input */ + if (Ctrl->CM4->CM4_D.dst == NULL) Ctrl->CM4->CM4_D.dst = calloc (1U, sizeof(double)); /* We need at least a size of one in case a value is given in input */ if (!Ctrl->A.fixed_time) /* Otherwise we don't print the time */ gmt_set_column_type (GMT, GMT_IO, t_col, GMT_IS_ABSTIME); diff --git a/test/mgd77/magref_dst.sh b/test/mgd77/magref_dst.sh new file mode 100755 index 00000000000..b8836f3fc91 --- /dev/null +++ b/test/mgd77/magref_dst.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +# +# gmt mgd77magref -D with a numeric Dst value used to segfault (the value was stored +# before its buffer was allocated) and dropped the sign of a negative value. + +rec="-30 45 0 2000.08700533" +echo "$rec" | gmt mgd77magref -A+y -Fxyz/3 -D8 > pos.txt || exit 1 +echo "$rec" | gmt mgd77magref -A+y -Fxyz/3 -D-8 > neg.txt || exit 1 +# The sign must matter and the output must be finite +cmp -s pos.txt neg.txt && { echo "-D-8 same as -D8: sign was dropped"; exit 1; } +test -s pos.txt && test -s neg.txt + +# With several records the single Dst value must apply to every row, not index past it +printf -- "-30 45 0 2000.08700533\n-30 45 0 2000.5\n-30 45 0 2001.2\n" > multi.txt +gmt mgd77magref multi.txt -A+y -Fxyz/3 -D8 > multi_all.txt || exit 1 +for i in 1 2 3; do + sed -n ${i}p multi.txt | gmt mgd77magref -A+y -Fxyz/3 -D8 +done > multi_one.txt || exit 1 +diff multi_all.txt multi_one.txt From 2fef6cae0b5f02f7126ae91bd68680ee1c6a54ad Mon Sep 17 00:00:00 2001 From: Esteban82 Date: Fri, 16 Oct 2026 15:17:43 -0300 Subject: [PATCH 2/4] Free the Dst buffer in Free_Ctrl so every exit path releases it Co-Authored-By: Claude Sonnet 5.5 --- src/mgd77/mgd77magref.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mgd77/mgd77magref.c b/src/mgd77/mgd77magref.c index 25f706413c8..60fba28c226 100644 --- a/src/mgd77/mgd77magref.c +++ b/src/mgd77/mgd77magref.c @@ -82,6 +82,7 @@ static void Free_Ctrl (struct GMT_CTRL *GMT, struct MGD77MAGREF_CTRL *C) { /* De if (!C) return; gmt_M_str_free (C->CM4->CM4_M.path); gmt_M_str_free (C->CM4->CM4_D.path); + gmt_M_str_free (C->CM4->CM4_D.dst); gmt_M_str_free (C->CM4->CM4_I.path); gmt_M_str_free (C->CM4); gmt_M_free (GMT, C); @@ -731,7 +732,6 @@ EXTERN_MSC int GMT_mgd77magref (void *V_API, int mode, void *args) { } gmt_M_free (GMT, Out); - gmt_M_str_free (Ctrl->CM4->CM4_D.dst); gmt_M_free (GMT, Ctrl->CM4->CM4_DATA.out_field); if (!(Ctrl->A.years || Ctrl->A.fixed_time)) gmt_M_free (GMT, time_years); if (Ctrl->joint_IGRF_CM4) gmt_M_free (GMT, igrf_xyz); From d0a443511c2181107da9e3e7da9abc97d512753f Mon Sep 17 00:00:00 2001 From: Esteban82 Date: Fri, 16 Oct 2026 17:26:18 -0300 Subject: [PATCH 3/4] Keep the Dst buffer when a segment falls outside the Dst table MGD77_cm4field freed it on that error, so the next single-record segment wrote into a NULL pointer. Free_Ctrl now owns the buffer. Co-Authored-By: Claude Opus 5.5 --- src/mgd77/cm4_functions.c | 1 - test/mgd77/magref_dst.sh | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/mgd77/cm4_functions.c b/src/mgd77/cm4_functions.c index 7896e2c0b10..0b72cb269bf 100644 --- a/src/mgd77/cm4_functions.c +++ b/src/mgd77/cm4_functions.c @@ -375,7 +375,6 @@ int MGD77_cm4field (struct GMT_CTRL *GMT, struct MGD77_CM4 *Ctrl, double *p_lon, if (dstx) Ctrl->CM4_D.dst[0] = intdst(mjdl, mjdh, mjdy[0], msec[0], dstx, &cerr); if (cerr > 49) { gmt_M_str_free (dstx); - if (Ctrl->CM4_DATA.n_times > 1) gmt_M_str_free (Ctrl->CM4_D.dst); clear_mem (mut, gpsq, gssq, gpmg, gsmg, hysq, epsq, essq, ecto, hyto, hq, ht, bkpo, ws, gamf, epmg, esmg, hymg, f107x, pleg, rcur, gcto_or, gcto_mg); gmt_M_str_free (msec); gmt_M_str_free (mjdy); free (mjdy); diff --git a/test/mgd77/magref_dst.sh b/test/mgd77/magref_dst.sh index b8836f3fc91..4798116daa3 100755 --- a/test/mgd77/magref_dst.sh +++ b/test/mgd77/magref_dst.sh @@ -17,3 +17,8 @@ for i in 1 2 3; do sed -n ${i}p multi.txt | gmt mgd77magref -A+y -Fxyz/3 -D8 done > multi_one.txt || exit 1 diff multi_all.txt multi_one.txt + +# A segment rejected for lying outside the Dst table must not break the next segment +printf -- "> bad\n-30 45 0 1900.5\n-30 45 0 1900.6\n> good\n-30 45 0 2000.5\n" > seg.txt +gmt mgd77magref seg.txt -A+y -Fxyz/3 -Vq > seg_out.txt || exit 1 +grep -v '^>' seg_out.txt | wc -l | grep -q '^ *1$' From 2e03144b3d5090f4b61d33249879c6bce16cd643 Mon Sep 17 00:00:00 2001 From: Esteban82 Date: Fri, 16 Oct 2026 23:26:38 -0300 Subject: [PATCH 4/4] Update test --- test/mgd77/magref_dst.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/mgd77/magref_dst.sh b/test/mgd77/magref_dst.sh index 4798116daa3..1f16ab677f0 100755 --- a/test/mgd77/magref_dst.sh +++ b/test/mgd77/magref_dst.sh @@ -8,7 +8,7 @@ echo "$rec" | gmt mgd77magref -A+y -Fxyz/3 -D8 > pos.txt || exit 1 echo "$rec" | gmt mgd77magref -A+y -Fxyz/3 -D-8 > neg.txt || exit 1 # The sign must matter and the output must be finite cmp -s pos.txt neg.txt && { echo "-D-8 same as -D8: sign was dropped"; exit 1; } -test -s pos.txt && test -s neg.txt +test -s pos.txt && test -s neg.txt || exit 1 # With several records the single Dst value must apply to every row, not index past it printf -- "-30 45 0 2000.08700533\n-30 45 0 2000.5\n-30 45 0 2001.2\n" > multi.txt @@ -16,7 +16,7 @@ gmt mgd77magref multi.txt -A+y -Fxyz/3 -D8 > multi_all.txt || exit 1 for i in 1 2 3; do sed -n ${i}p multi.txt | gmt mgd77magref -A+y -Fxyz/3 -D8 done > multi_one.txt || exit 1 -diff multi_all.txt multi_one.txt +diff multi_all.txt multi_one.txt || exit 1 # A segment rejected for lying outside the Dst table must not break the next segment printf -- "> bad\n-30 45 0 1900.5\n-30 45 0 1900.6\n> good\n-30 45 0 2000.5\n" > seg.txt