Package: posh
Version: 0.12.3
X-Debbugs-CC: j...@thesalmons.org
The initialization of the PWD variable has an off-by-one error in its
length calculation. The current_wd_size is not sufficient to hold the
terminal NUL. Since the contents of current_wd is not NUL-terminated,
subsequent use of that variable could result in potentially dangerous
buffer overruns.
I am not aware of any externally visible or exploitable behavior, but
valgrind reports the error and it's obvious by inspection.
The patch is easy:
drdlogin0039$ git diff master fixpwdlen
diff --git a/main.c b/main.c
index a2ea74f..88a3a5b 100644
--- a/main.c
+++ b/main.c
@@ -199,7 +199,7 @@ main(int argc, char **argv)
int len;
simplified = canonicalize_file_name(current_wd);
- len = strlen(simplified);
+ len = strlen(simplified) + 1;
if (len > current_wd_size)
current_wd = aresize(current_wd, current_wd_size =
len, APERM);
drdlogin0039$
Herre is some valgrind output:
First, the unpatched code:
drdlogin0039$ git checkout master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
drdlogin0039$ make
make all-recursive
make[1]: Entering directory `/d/en/salmonj-3/g/posh'
Making all in .
make[2]: Entering directory `/d/en/salmonj-3/g/posh'
gcc -DHAVE_CONFIG_H -I. -g -O2 -MT main.o -MD -MP -MF .deps/main.Tpo
-c -o main.o main.c
mv -f .deps/main.Tpo .deps/main.Po
gcc -g -O2 -o posh alloc.o c_test.o eval.o exec.o expr.o history.o
io.o jobs.o lex.o main.o misc.o path.o shf.o syn.o table.o trap.o tree.o
tty.o var.o builtins.o compat.o times.o
make[2]: Leaving directory `/d/en/salmonj-3/g/posh'
Making all in src
make[2]: Entering directory `/d/en/salmonj-3/g/posh/src'
make[2]: Nothing to be done for `all'.
make[2]: Leaving directory `/d/en/salmonj-3/g/posh/src'
Making all in tests
make[2]: Entering directory `/d/en/salmonj-3/g/posh/tests'
make[2]: Nothing to be done for `all'.
make[2]: Leaving directory `/d/en/salmonj-3/g/posh/tests'
make[1]: Leaving directory `/d/en/salmonj-3/g/posh'
drdlogin0039$ valgrind ./posh -c 'exit 0'
==29822== Memcheck, a memory error detector
==29822== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==29822== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==29822== Command: ./posh -c exit\ 0
==29822==
==29822== Invalid read of size 1
==29822== at 0x4A07FA4: strlen (mc_replace_strmem.c:403)
==29822== by 0x414235: export (var.c:528)
==29822== by 0x414ACA: setstr (var.c:376)
==29822== by 0x401F87: main (main.c:215)
==29822== Address 0x4c4f03e is 0 bytes after a block of size 30 alloc'd
==29822== at 0x4A06C20: realloc (vg_replace_malloc.c:662)
==29822== by 0x4026F5: aresize (alloc.c:76)
==29822== by 0x402433: main (main.c:205)
==29822==
==29822== Invalid read of size 1
==29822== at 0x4A08DEC: memcpy (mc_replace_strmem.c:882)
==29822== by 0x414288: export (var.c:536)
==29822== by 0x414ACA: setstr (var.c:376)
==29822== by 0x401F87: main (main.c:215)
==29822== Address 0x4c4f03e is 0 bytes after a block of size 30 alloc'd
==29822== at 0x4A06C20: realloc (vg_replace_malloc.c:662)
==29822== by 0x4026F5: aresize (alloc.c:76)
==29822== by 0x402433: main (main.c:205)
==29822==
==29822==
==29822== HEAP SUMMARY:
==29822== in use at exit: 21,337 bytes in 375 blocks
==29822== total heap usage: 698 allocs, 323 frees, 43,119 bytes allocated
==29822==
==29822== LEAK SUMMARY:
==29822== definitely lost: 32 bytes in 1 blocks
==29822== indirectly lost: 2,848 bytes in 89 blocks
==29822== possibly lost: 0 bytes in 0 blocks
==29822== still reachable: 18,457 bytes in 285 blocks
==29822== suppressed: 0 bytes in 0 blocks
==29822== Rerun with --leak-check=full to see details of leaked memory
==29822==
==29822== For counts of detected and suppressed errors, rerun with: -v
==29822== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 6 from 6)
And now recompile and rerun valgrind on the patched branch:
drdlogin0039$ git checkout fixpwdlen
Switched to branch 'fixpwdlen'
drdlogin0039$ make
make all-recursive
make[1]: Entering directory `/d/en/salmonj-3/g/posh'
Making all in .
make[2]: Entering directory `/d/en/salmonj-3/g/posh'
gcc -DHAVE_CONFIG_H -I. -g -O2 -MT main.o -MD -MP -MF .deps/main.Tpo
-c -o main.o main.c
mv -f .deps/main.Tpo .deps/main.Po
gcc -g -O2 -o posh alloc.o c_test.o eval.o exec.o expr.o history.o
io.o jobs.o lex.o main.o misc.o path.o shf.o syn.o table.o trap.o tree.o
tty.o var.o builtins.o compat.o times.o
make[2]: Leaving directory `/d/en/salmonj-3/g/posh'
Making all in src
make[2]: Entering directory `/d/en/salmonj-3/g/posh/src'
make[2]: Nothing to be done for `all'.
make[2]: Leaving directory `/d/en/salmonj-3/g/posh/src'
Making all in tests
make[2]: Entering directory `/d/en/salmonj-3/g/posh/tests'
make[2]: Nothing to be done for `all'.
make[2]: Leaving directory `/d/en/salmonj-3/g/posh/tests'
make[1]: Leaving directory `/d/en/salmonj-3/g/posh'
drdlogin0039$ valgrind ./posh -c 'exit 0'
==30073== Memcheck, a memory error detector
==30073== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==30073== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==30073== Command: ./posh -c exit\ 0
==30073==
==30073==
==30073== HEAP SUMMARY:
==30073== in use at exit: 21,338 bytes in 375 blocks
==30073== total heap usage: 698 allocs, 323 frees, 43,120 bytes allocated
==30073==
==30073== LEAK SUMMARY:
==30073== definitely lost: 32 bytes in 1 blocks
==30073== indirectly lost: 2,848 bytes in 89 blocks
==30073== possibly lost: 0 bytes in 0 blocks
==30073== still reachable: 18,458 bytes in 285 blocks
==30073== suppressed: 0 bytes in 0 blocks
==30073== Rerun with --leak-check=full to see details of leaked memory
==30073==
==30073== For counts of detected and suppressed errors, rerun with: -v
==30073== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 6)
drdlogin0039$
--
*.*
