hc
2024-03-22 619f0f87159c5dbd2755b1b0a0eb35784be84e7a
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
From 4ea7bae51f97e49c84dc67ea30b466ca8633b9f6 Mon Sep 17 00:00:00 2001
From: Chris Coulson <chris.coulson@canonical.com>
Date: Thu, 7 Jan 2021 19:21:03 +0000
Subject: [PATCH] kern/parser: Fix a stack buffer overflow
 
grub_parser_split_cmdline() expands variable names present in the supplied
command line in to their corresponding variable contents and uses a 1 kiB
stack buffer for temporary storage without sufficient bounds checking. If
the function is called with a command line that references a variable with
a sufficiently large payload, it is possible to overflow the stack
buffer via tab completion, corrupt the stack frame and potentially
control execution.
 
Fixes: CVE-2020-27749
 
Reported-by: Chris Coulson <chris.coulson@canonical.com>
Signed-off-by: Chris Coulson <chris.coulson@canonical.com>
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com>
---
 grub-core/kern/parser.c | 110 +++++++++++++++++++++++++++++-------------------
 1 file changed, 67 insertions(+), 43 deletions(-)
 
diff --git a/grub-core/kern/parser.c b/grub-core/kern/parser.c
index e010eaa..6ab7aa4 100644
--- a/grub-core/kern/parser.c
+++ b/grub-core/kern/parser.c
@@ -18,6 +18,7 @@
  */
 
 #include <grub/parser.h>
+#include <grub/buffer.h>
 #include <grub/env.h>
 #include <grub/misc.h>
 #include <grub/mm.h>
@@ -107,8 +108,8 @@ check_varstate (grub_parser_state_t s)
 }
 
 
-static void
-add_var (char *varname, char **bp, char **vp,
+static grub_err_t
+add_var (grub_buffer_t varname, grub_buffer_t buf,
      grub_parser_state_t state, grub_parser_state_t newstate)
 {
   const char *val;
@@ -116,31 +117,41 @@ add_var (char *varname, char **bp, char **vp,
   /* Check if a variable was being read in and the end of the name
      was reached.  */
   if (!(check_varstate (state) && !check_varstate (newstate)))
-    return;
+    return GRUB_ERR_NONE;
+
+  if (grub_buffer_append_char (varname, '\0') != GRUB_ERR_NONE)
+    return grub_errno;
 
-  *((*vp)++) = '\0';
-  val = grub_env_get (varname);
-  *vp = varname;
+  val = grub_env_get ((const char *) grub_buffer_peek_data (varname));
+  grub_buffer_reset (varname);
   if (!val)
-    return;
+    return GRUB_ERR_NONE;
 
   /* Insert the contents of the variable in the buffer.  */
-  for (; *val; val++)
-    *((*bp)++) = *val;
+  return grub_buffer_append_data (buf, val, grub_strlen (val));
 }
 
-static void
-terminate_arg (char *buffer, char **bp, int *argc)
+static grub_err_t
+terminate_arg (grub_buffer_t buffer, int *argc)
 {
-  if (*bp != buffer && *((*bp) - 1) != '\0')
-    {
-      *((*bp)++) = '\0';
-      (*argc)++;
-    }
+  grub_size_t unread = grub_buffer_get_unread_bytes (buffer);
+
+  if (unread == 0)
+    return GRUB_ERR_NONE;
+
+  if (*(const char *) grub_buffer_peek_data_at (buffer, unread - 1) == '\0')
+    return GRUB_ERR_NONE;
+
+  if (grub_buffer_append_char (buffer, '\0') != GRUB_ERR_NONE)
+    return grub_errno;
+
+  (*argc)++;
+
+  return GRUB_ERR_NONE;
 }
 
 static grub_err_t
-process_char (char c, char *buffer, char **bp, char *varname, char **vp,
+process_char (char c, grub_buffer_t buffer, grub_buffer_t varname,
           grub_parser_state_t state, int *argc,
           grub_parser_state_t *newstate)
 {
@@ -153,12 +164,13 @@ process_char (char c, char *buffer, char **bp, char *varname, char **vp,
    * not describe the variable anymore, write the variable to
    * the buffer.
    */
-  add_var (varname, bp, vp, state, *newstate);
+  if (add_var (varname, buffer, state, *newstate) != GRUB_ERR_NONE)
+    return grub_errno;
 
   if (check_varstate (*newstate))
     {
       if (use)
-    *((*vp)++) = use;
+        return grub_buffer_append_char (varname, use);
     }
   else if (*newstate == GRUB_PARSER_STATE_TEXT &&
        state != GRUB_PARSER_STATE_ESC && grub_isspace (use))
@@ -167,10 +179,10 @@ process_char (char c, char *buffer, char **bp, char *varname, char **vp,
        * Don't add more than one argument if multiple
        * spaces are used.
        */
-      terminate_arg (buffer, bp, argc);
+      return terminate_arg (buffer, argc);
     }
   else if (use)
-    *((*bp)++) = use;
+    return grub_buffer_append_char (buffer, use);
 
   return GRUB_ERR_NONE;
 }
@@ -181,19 +193,22 @@ grub_parser_split_cmdline (const char *cmdline,
                int *argc, char ***argv)
 {
   grub_parser_state_t state = GRUB_PARSER_STATE_TEXT;
-  /* XXX: Fixed size buffer, perhaps this buffer should be dynamically
-     allocated.  */
-  char buffer[1024];
-  char *bp = buffer;
+  grub_buffer_t buffer, varname;
   char *rd = (char *) cmdline;
   char *rp = rd;
-  char varname[200];
-  char *vp = varname;
-  char *args;
   int i;
 
   *argc = 0;
   *argv = NULL;
+
+  buffer = grub_buffer_new (1024);
+  if (buffer == NULL)
+    return grub_errno;
+
+  varname = grub_buffer_new (200);
+  if (varname == NULL)
+    goto fail;
+
   do
     {
       if (rp == NULL || *rp == '\0')
@@ -219,7 +234,7 @@ grub_parser_split_cmdline (const char *cmdline,
     {
       grub_parser_state_t newstate;
 
-      if (process_char (*rp, buffer, &bp, varname, &vp, state, argc,
+      if (process_char (*rp, buffer, varname, state, argc,
                 &newstate) != GRUB_ERR_NONE)
         goto fail;
 
@@ -230,10 +245,12 @@ grub_parser_split_cmdline (const char *cmdline,
 
   /* A special case for when the last character was part of a
      variable.  */
-  add_var (varname, &bp, &vp, state, GRUB_PARSER_STATE_TEXT);
+  if (add_var (varname, buffer, state, GRUB_PARSER_STATE_TEXT) != GRUB_ERR_NONE)
+    goto fail;
 
   /* Ensure that the last argument is terminated. */
-  terminate_arg (buffer, &bp, argc);
+  if (terminate_arg (buffer, argc) != GRUB_ERR_NONE)
+    goto fail;
 
   /* If there are no args, then we're done. */
   if (!*argc)
@@ -242,38 +259,45 @@ grub_parser_split_cmdline (const char *cmdline,
       goto out;
     }
 
-  /* Reserve memory for the return values.  */
-  args = grub_malloc (bp - buffer);
-  if (!args)
-    goto fail;
-  grub_memcpy (args, buffer, bp - buffer);
-
   *argv = grub_calloc (*argc + 1, sizeof (char *));
   if (!*argv)
     goto fail;
 
   /* The arguments are separated with 0's, setup argv so it points to
      the right values.  */
-  bp = args;
   for (i = 0; i < *argc; i++)
     {
-      (*argv)[i] = bp;
-      while (*bp)
-    bp++;
-      bp++;
+      char *arg;
+
+      if (i > 0)
+    {
+      if (grub_buffer_advance_read_pos (buffer, 1) != GRUB_ERR_NONE)
+        goto fail;
+    }
+
+      arg = (char *) grub_buffer_peek_data (buffer);
+      if (arg == NULL ||
+      grub_buffer_advance_read_pos (buffer, grub_strlen (arg)) != GRUB_ERR_NONE)
+    goto fail;
+
+      (*argv)[i] = arg;
     }
 
+  /* Keep memory for the return values. */
+  grub_buffer_take_data (buffer);
+
   grub_errno = GRUB_ERR_NONE;
 
  out:
   if (rd != cmdline)
     grub_free (rd);
+  grub_buffer_free (buffer);
+  grub_buffer_free (varname);
 
   return grub_errno;
 
  fail:
   grub_free (*argv);
-  grub_free (args);
   goto out;
 }
 
-- 
2.14.2