* [U-Boot] [PATCH 1/2] FIX: env: import: hashtable: Prevent buffer overrun when importing environment from file
@ 2015-09-13 22:57 Lukasz Majewski
2015-09-13 22:57 ` [U-Boot] [PATCH 2/2] FIX: env: import: hashtable: Free memory allocated before exiting from himport_r() Lukasz Majewski
2015-09-15 21:34 ` [U-Boot] [U-Boot, 1/2] FIX: env: import: hashtable: Prevent buffer overrun when importing environment from file Tom Rini
0 siblings, 2 replies; 4+ messages in thread
From: Lukasz Majewski @ 2015-09-13 22:57 UTC (permalink / raw)
To: u-boot
Lets consider following scenario:
- One uses echo -n "key=value" to define environment variable in a file (single variable)
- The file content is "key=value" without any terminating byte (e.g. 0x0a or
0x0d).
- The file is loaded to u-boot non zero'ed RAM buffer (with load command).
- Then "env import -t -r $loadaddr $filesize" is executed.
- Due to lack of proper termination byte we have classical example of buffer
overrun.
This patch prevents from this by allocating one extra byte than size and
explicitly null terminate it.
There should be no change for normal env import operation after applying
this patch.
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
lib/hashtable.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 18ed590..7df424a 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -789,12 +789,13 @@ int himport_r(struct hsearch_data *htab,
}
/* we allocate new space to make sure we can write to the array */
- if ((data = malloc(size)) == NULL) {
- debug("himport_r: can't malloc %zu bytes\n", size);
+ if ((data = malloc(size + 1)) == NULL) {
+ debug("himport_r: can't malloc %zu bytes\n", size + 1);
__set_errno(ENOMEM);
return 0;
}
memcpy(data, env, size);
+ data[size] = '\0';
dp = data;
/* make a local copy of the list of variables */
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* [U-Boot] [PATCH 2/2] FIX: env: import: hashtable: Free memory allocated before exiting from himport_r()
2015-09-13 22:57 [U-Boot] [PATCH 1/2] FIX: env: import: hashtable: Prevent buffer overrun when importing environment from file Lukasz Majewski
@ 2015-09-13 22:57 ` Lukasz Majewski
2015-09-15 21:34 ` [U-Boot] [U-Boot, " Tom Rini
2015-09-15 21:34 ` [U-Boot] [U-Boot, 1/2] FIX: env: import: hashtable: Prevent buffer overrun when importing environment from file Tom Rini
1 sibling, 1 reply; 4+ messages in thread
From: Lukasz Majewski @ 2015-09-13 22:57 UTC (permalink / raw)
To: u-boot
Without this patch memory is not released on early exit.
Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
---
lib/hashtable.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/hashtable.c b/lib/hashtable.c
index 7df424a..02b4105 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -842,8 +842,10 @@ int himport_r(struct hsearch_data *htab,
}
}
- if(!size)
+ if (!size) {
+ free(data);
return 1; /* everything OK */
+ }
if(crlf_is_lf) {
/* Remove Carriage Returns in front of Line Feeds */
unsigned ignored_crs = 0;
@@ -907,6 +909,7 @@ int himport_r(struct hsearch_data *htab,
if (*name == 0) {
debug("INSERT: unable to use an empty key\n");
__set_errno(EINVAL);
+ free(data);
return 0;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* [U-Boot] [U-Boot, 1/2] FIX: env: import: hashtable: Prevent buffer overrun when importing environment from file
2015-09-13 22:57 [U-Boot] [PATCH 1/2] FIX: env: import: hashtable: Prevent buffer overrun when importing environment from file Lukasz Majewski
2015-09-13 22:57 ` [U-Boot] [PATCH 2/2] FIX: env: import: hashtable: Free memory allocated before exiting from himport_r() Lukasz Majewski
@ 2015-09-15 21:34 ` Tom Rini
1 sibling, 0 replies; 4+ messages in thread
From: Tom Rini @ 2015-09-15 21:34 UTC (permalink / raw)
To: u-boot
On Mon, Sep 14, 2015 at 12:57:03AM +0200, Lukasz Majewski wrote:
> Lets consider following scenario:
> - One uses echo -n "key=value" to define environment variable in a file (single variable)
> - The file content is "key=value" without any terminating byte (e.g. 0x0a or
> 0x0d).
> - The file is loaded to u-boot non zero'ed RAM buffer (with load command).
> - Then "env import -t -r $loadaddr $filesize" is executed.
> - Due to lack of proper termination byte we have classical example of buffer
> overrun.
>
> This patch prevents from this by allocating one extra byte than size and
> explicitly null terminate it.
>
> There should be no change for normal env import operation after applying
> this patch.
>
> Signed-off-by: Lukasz Majewski <l.majewski@majess.pl>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150915/695a3aa9/attachment.sig>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-09-15 21:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-13 22:57 [U-Boot] [PATCH 1/2] FIX: env: import: hashtable: Prevent buffer overrun when importing environment from file Lukasz Majewski
2015-09-13 22:57 ` [U-Boot] [PATCH 2/2] FIX: env: import: hashtable: Free memory allocated before exiting from himport_r() Lukasz Majewski
2015-09-15 21:34 ` [U-Boot] [U-Boot, " Tom Rini
2015-09-15 21:34 ` [U-Boot] [U-Boot, 1/2] FIX: env: import: hashtable: Prevent buffer overrun when importing environment from file Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox