[coreboot-gerrit] Change in coreboot[master]: amdfwtool: Check the length of matching string before accessing

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62483 )

Change subject: amdfwtool: Check the length of matching string before accessing
......................................................................

amdfwtool: Check the length of matching string before accessing

If AB recovery is enabled and get a "Lx" in fw.cfg, wrong character
is got or access violation happens.

Change-Id: Ibd8ffe34fd44d860ec2115cd36117da7b02169cd
Signed-off-by: Zheng Bao 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/62483
Tested-by: build bot (Jenkins) 
Reviewed-by: Felix Held 
Reviewed-by: Jason Glenesk 
---
M util/amdfwtool/data_parse.c
1 file changed, 21 insertions(+), 10 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Felix Held: Looks good to me, approved
  Jason Glenesk: Looks good to me, but someone else must approve

diff --git a/util/amdfwtool/data_parse.c b/util/amdfwtool/data_parse.c
index 7f1bc3a..e883d17 100644
--- a/util/amdfwtool/data_parse.c
+++ b/util/amdfwtool/data_parse.c
@@ -472,6 +472,25 @@
    return retval;
 }

+char get_level_from_config(char *line, regoff_t level_index, amd_cb_config *cb_config)
+{
+	char lvl = 'x';
+	/* If the optional level field is present,
+	   extract the level char. */
+	if (level_index != -1) {
+		if (cb_config->recovery_ab == 0)
+			lvl = line[level_index + 1];
+		else if (strlen(&line[level_index]) >= 3)
+			lvl = line[level_index + 2];
+	}
+
+	assert(lvl == 'x' || lvl == 'X' ||
+		lvl == 'b' || lvl == 'B' ||
+		lvl == '1' || lvl == '2');
+
+	return lvl;
+}
+
 /*
   return value:
    0: The config file can not be parsed correctly.
@@ -537,16 +556,8 @@

/* If the optional level field is present,
    			   extract the level char. */
-				if (match[3].rm_so != -1) {
-					if (cb_config->recovery_ab == 0)
-						ch_lvl = oneline[match[3].rm_so + 1];
-					else
-						ch_lvl = oneline[match[3].rm_so + 2];
-				}
-
-				assert(ch_lvl == 'x' || ch_lvl == 'X' ||
-					ch_lvl == 'b' || ch_lvl == 'B' ||
-					ch_lvl == '1' || ch_lvl == '2');
+				ch_lvl = get_level_from_config(oneline,
+						match[3].rm_so, cb_config);

if (find_register_fw_filename_psp_dir(
    					&(oneline[match[1].rm_so]),