psql: avoid over-reading unterminated prompt escapes - Mailing list pgsql-hackers

Hi,

I happened to encounter a psql crash that could not be reproduced reliably with a normal build. However,
AddressSanitizerreproduces it consistently. 

1. Build psql with AddressSanitizer
```
% ./configure CFLAGS='-O1 -g -fsanitize=address -fno-omit-frame-pointer' LDFLAGS='-fsanitize=address’
% make -C src/bin/psql psql
```
Note: On my MacBook, gcc points to clang.

2. Run psql and set PROMPT1 to an unterminated variable
```
evantest=# \set PROMPT1 '%:aa'
=================================================================
==34121==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000001f35 at pc 0x0001024be288 bp 0x00016d976930
sp0x00016d976928 
READ of size 1 at 0x602000001f35 thread T0
    #0 0x0001024be284 in get_prompt prompt.c:103
    #1 0x0001024bc1a8 in MainLoop mainloop.c:166
    #2 0x0001024ce05c in main startup.c:471
    #3 0x0001827ac4e0 in start+0x1b4c (dyld:arm64e+0x204e0)

0x602000001f35 is located 0 bytes after 5-byte region [0x602000001f30,0x602000001f35)
allocated by thread T0 here:
    #0 0x000103172b54 in strdup+0x108 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ab54)
    #1 0x0001025080e4 in pg_strdup fe_memutils.c:101
    #2 0x0001024dcf5c in SetVariable variables.c:316
    #3 0x0001024952e4 in exec_command_set command.c:2923
    #4 0x00010248b8b8 in exec_command command.c:445
    #5 0x0001024889e4 in HandleSlashCmds command.c:260
    #6 0x0001024bcb70 in MainLoop mainloop.c:499
    #7 0x0001024ce05c in main startup.c:471
    #8 0x0001827ac4e0 in start+0x1b4c (dyld:arm64e+0x204e0)

SUMMARY: AddressSanitizer: heap-buffer-overflow prompt.c:103 in get_prompt
Shadow bytes around the buggy address:
  0x602000001c80: fa fa 03 fa fa fa 02 fa fa fa 00 03 fa fa 06 fa
  0x602000001d00: fa fa 02 fa fa fa 00 02 fa fa 00 02 fa fa 00 05
  0x602000001d80: fa fa 00 02 fa fa 00 07 fa fa 02 fa fa fa 00 02
  0x602000001e00: fa fa 00 02 fa fa 00 02 fa fa 00 02 fa fa 00 02
  0x602000001e80: fa fa 00 07 fa fa 00 fa fa fa 00 04 fa fa fd fa
=>0x602000001f00: fa fa fd fa fa fa[05]fa fa fa fd fa fa fa fa fa
  0x602000001f80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x602000002000: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x602000002080: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x602000002100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x602000002180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==34121==ABORTING
zsh: abort      psql -d evantest
```

The problem is that the current code assumes a terminating “:" exists. When it does, "p += nameend + 1" makes p point
tothe terminating colon, and the for loop's increment advances p to the string's terminating '\0'. When the terminating
colonis absent, the same assignment already makes p point to '\0', and the for loop's increment advances p one past the
endof the string. The next loop condition then dereferences p out of bounds. If that invalid read yields a nonzero
value,the loop continues and can perform further out-of-bounds reads. 

The fix is straightforward, only advance over the terminating colon when it exists. The same problem also exists for
the%\command`` escape. 

See the attached patch for details.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/