Skip to content

Reject a blank row in the gencost matrix instead of reading past the string - #580

Closed
arpitjain099 wants to merge 1 commit into
ORNL:developfrom
arpitjain099:fix/gencost-blank-row
Closed

arpitjain099 wants to merge 1 commit into
ORNL:developfrom
arpitjain099:fix/gencost-blank-row

Conversation

@arpitjain099

Copy link
Copy Markdown

readMatPowerGenCostRow checks the trailing semicolon with row[row.size() - 1] after rtrim. On a blank line the string is empty, size() - 1 wraps, and the subscript reads past the buffer. The gencost loop in readMatPower hands every line up to ]; to the row reader without filtering empty ones, so a case file with a stray blank line between two gencost rows reaches it.

Built with -fsanitize=address against the header as it stands:

control (no blank line): parsed, rows=2
ERROR: AddressSanitizer: stack-buffer-underflow
  #0 readMatPowerGenCostRow MatpowerParser.hpp:156
  #1 readMatPower MatpowerParser.hpp:282

The file that triggers it is a normal case file, nothing malicious, just an extra newline in the matrix.

The condition already throws matlab_syntax_error for a row without a trailing semicolon, so adding row.empty() to it puts a blank row on that same path rather than inventing new behaviour. I added the case to test_parse_gencost_row, which aborts under ASan before the change and passes after it.

One thing I noticed while checking the siblings, not changed here: readMatPowerGenRow and readMatPowerBranchRow both end with checkEndOfMatrixRow, so they reject a blank row, but readMatPowerBusRow does not call it. A blank line in mpc.bus is currently accepted and appends a bus row with every field zero. Happy to send that separately if you want it tightened up, since it changes what the bus parser accepts rather than fixing a read.

…string

readMatPowerGenCostRow checks the last character with row[row.size() - 1]
after rtrim. On a blank line inside mpc.gencost that subscript is
row[std::string::npos], since size() is 0 and the expression is unsigned.

The read is reachable from readMatPower: the gencost loop passes every line
up to "];" straight to the row reader, and nothing filters empty ones.
Compiled with -fsanitize=address, a case file with one blank line between two
gencost rows gives:

  ERROR: AddressSanitizer: stack-buffer-underflow
  #0 readMatPowerGenCostRow MatpowerParser.hpp:156
  ORNL#1 readMatPower MatpowerParser.hpp:282

while the same file without the blank line parses both rows.

Added the empty check to the condition that was already there, so a blank row
now raises the same matlab_syntax_error any other malformed row does, and
added the case to test_parse_gencost_row.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
@pelesh

pelesh commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

@arpitjain099, thanks for your contribution. Please create an issue with reproducer and suggested solution. Then we can decide how to move forward. I will close this PR until then.

@pelesh pelesh closed this Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants