-
Notifications
You must be signed in to change notification settings - Fork 203
fix: validate row type byte before enum cast in MPS parser #1491
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -222,6 +222,24 @@ ObjSenseType convert_to_obj_sense(const std::string& str) | |||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| RowType convert_to_row_type(char c) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| switch (c) { | ||||||||||||||||||||||||||||||||
| case 'N': return Objective; | ||||||||||||||||||||||||||||||||
| case 'L': return LesserThanOrEqual; | ||||||||||||||||||||||||||||||||
| case 'G': return GreaterThanOrEqual; | ||||||||||||||||||||||||||||||||
| case 'E': return Equality; | ||||||||||||||||||||||||||||||||
| default: | ||||||||||||||||||||||||||||||||
| mps_parser_expects( | ||||||||||||||||||||||||||||||||
| false, | ||||||||||||||||||||||||||||||||
| error_type_t::ValidationError, | ||||||||||||||||||||||||||||||||
| "Invalid row type '%c' (0x%02x) found in ROWS section; expected N, L, G, or E", | ||||||||||||||||||||||||||||||||
| (c >= 0x20 && c < 0x7f) ? c : '?', | ||||||||||||||||||||||||||||||||
| static_cast<unsigned char>(c)); | ||||||||||||||||||||||||||||||||
| return Objective; // unreachable; mps_parser_expects throws | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| template <typename i_t, typename f_t> | ||||||||||||||||||||||||||||||||
| void mps_parser_t<i_t, f_t>::fill_problem(mps_data_model_t<i_t, f_t>& problem) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
|
|
@@ -785,13 +803,13 @@ void mps_parser_t<i_t, f_t>::parse_rows(std::string_view line) | |||||||||||||||||||||||||||||||
| std::string name; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| if (fixed_mps_format) { | ||||||||||||||||||||||||||||||||
| type = static_cast<RowType>(line[1]); | ||||||||||||||||||||||||||||||||
| type = convert_to_row_type(line[1]); | ||||||||||||||||||||||||||||||||
| name = trim(line.substr(4, 8)); // max of 8 chars allowed | ||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||
| std::stringstream ss{std::string(line)}; | ||||||||||||||||||||||||||||||||
| char read_word; | ||||||||||||||||||||||||||||||||
| ss >> read_word; | ||||||||||||||||||||||||||||||||
| type = static_cast<RowType>(read_word); | ||||||||||||||||||||||||||||||||
| type = convert_to_row_type(read_word); | ||||||||||||||||||||||||||||||||
|
Comment on lines
809
to
+812
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Parse the free-format row type as a token, not a single
Suggested fix } else {
std::stringstream ss{std::string(line)};
- char read_word;
- ss >> read_word;
- type = convert_to_row_type(read_word);
+ std::string row_type_token;
+ ss >> row_type_token;
+ mps_parser_expects(!row_type_token.empty() && row_type_token.size() == 1,
+ error_type_t::ValidationError,
+ "Invalid row type '%s' found in ROWS section; expected N, L, G, or E",
+ row_type_token.c_str());
+ type = convert_to_row_type(row_type_token[0]);
ss >> name;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| ss >> name; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard fixed-format
ROWSlines before indexing.line[1]andline.substr(4, 8)run before any validation, so a short malformed ROWS record can still hit undefined behavior or throwstd::out_of_rangeinstead of the intendedValidationError.Suggested fix
if (fixed_mps_format) { + mps_parser_expects(line.size() >= 5, + error_type_t::ValidationError, + "ROWS should have at least a row type and name! line=%s", + std::string(line).c_str()); type = convert_to_row_type(line[1]); name = trim(line.substr(4, 8)); // max of 8 chars allowed } else {📝 Committable suggestion
🤖 Prompt for AI Agents