psql: avoid over-reading unterminated prompt escapes

Started by Chao Liabout 2 months ago4 messageshackers
Jump to latest
#1Chao Li
li.evan.chao@gmail.com

Hi,

I happened to encounter a psql crash that could not be reproduced reliably with a normal build. However, AddressSanitizer reproduces 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 sp 0x00016d976928
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 to the terminating colon, and the for loop's increment advances p to the string's terminating '\0'. When the terminating colon is absent, the same assignment already makes p point to '\0', and the for loop's increment advances p one past the end of 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/

Attachments:

t253874_1
v1-0001-psql-Avoid-over-reading-unterminated-prompt-escap.patchapplication/octet-stream; name=v1-0001-psql-Avoid-over-reading-unterminated-prompt-escap.patch; x-unix-mode=0644Download+6-3
#2Kirill Reshke
reshkekirill@gmail.com
In reply to: Chao Li (#1)
Re: psql: avoid over-reading unterminated prompt escapes

On Mon, 21 Sept 2026 at 08:31, Chao Li <li.evan.chao@gmail.com> wrote:

Hi,

I happened to encounter a psql crash that could not be reproduced reliably with a normal build. However, AddressSanitizer reproduces 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 sp 0x00016d976928
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 to the terminating colon, and the for loop's increment advances p to the string's terminating '\0'. When the terminating colon is absent, the same assignment already makes p point to '\0', and the for loop's increment advances p one past the end of 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/

Hi!
I think this is indeed a real issue, please register CF item for this.
Looks like this code is dating back to a45195a191 [0]https://github.com/postgres/postgres/blob/a45195a191eec367a4f305bb71ab541d17a3b9f9/src/bin/psql/prompt.c#L227, so this bug
exists in all supported versions.

Code fix itself is fine but maybe write it like `if p[1] != NULL` for
consistency.

[0]: https://github.com/postgres/postgres/blob/a45195a191eec367a4f305bb71ab541d17a3b9f9/src/bin/psql/prompt.c#L227

--
Best regards,
Kirill Reshke

#3Chao Li
li.evan.chao@gmail.com
In reply to: Kirill Reshke (#2)
Re: psql: avoid over-reading unterminated prompt escapes

On Sep 21, 2026, at 13:04, Kirill Reshke <reshkekirill@gmail.com> wrote:

On Mon, 21 Sept 2026 at 08:31, Chao Li <li.evan.chao@gmail.com> wrote:

Hi,

I happened to encounter a psql crash that could not be reproduced reliably with a normal build. However, AddressSanitizer reproduces 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 sp 0x00016d976928
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 to the terminating colon, and the for loop's increment advances p to the string's terminating '\0'. When the terminating colon is absent, the same assignment already makes p point to '\0', and the for loop's increment advances p one past the end of 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/

Hi!
I think this is indeed a real issue, please register CF item for this.
Looks like this code is dating back to a45195a191 [0], so this bug
exists in all supported versions.

Thank you very much for the review. Yes, this is an old bug.

Code fix itself is fine but maybe write it like `if p[1] != NULL` for
consistency.

Make sense. We should use ‘\0’ instead of NULL.

[0] https://github.com/postgres/postgres/blob/a45195a191eec367a4f305bb71ab541d17a3b9f9/src/bin/psql/prompt.c#L227

--
Best regards,
Kirill Reshke

PFA v2 - addressed Kirill’s comment.

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

Attachments:

t253874_3
v2-0001-psql-Avoid-over-reading-unterminated-prompt-escap.patchapplication/octet-stream; name=v2-0001-psql-Avoid-over-reading-unterminated-prompt-escap.patch; x-unix-mode=0644Download+6-3
#4Ishjaap Singh
ishjaap.singh07@gmail.com
In reply to: Chao Li (#3)
Re: psql: avoid over-reading unterminated prompt escapes

On Mon, Sep 21, 2026 at 07:35, Chao Li wrote:

PFA v2 - addressed Kirill's comment.

Hi Chao,

I reviewed and tested v2.

I reproduced the over-read on master (0019a1ecf4c) with an
AddressSanitizer build. Without the patch, each of these prompts
triggers a heap-buffer-overflow at prompt.c:103:

%:aa %:x %: pre %:aa %`echo hi %`

With v2 applied, all of them run cleanly, and well-formed prompts
(%:aa:, %`echo hi`, %/%R%# and a few others) render exactly as
before. The psql TAP tests and the core regression tests also pass
under ASan.

The fix looks correct to me. Since strcspn() stops at either the
delimiter or the terminating NUL, p[1] is always one of the two, so
the pointer can no longer step past the end of the string.

For the back branches, v2 doesn't apply as-is because master changed
free() to pfree() next to these lines in 30652b356d. The attached
v2-backbranch-REL_14-19.patch.txt adjusts only that context; the
code change itself is identical. It applies to REL_14_STABLE through
REL_19_STABLE. I built and tested REL_14_STABLE with ASan: the same
six cases fail before the fix and run cleanly after it.

I've also attached an optional TAP test (0002, on top of v2), since
nothing currently exercises custom prompts. It passes with the fix,
and without it ASan aborts psql at the first unterminated case. In a
normal build it passed both with and without the fix, so it only
reliably catches this under ASan. Feel free to leave it out if you
don't think it's worth adding.

This looks ready for committer to me.

Thanks,
Ishjaap Singh

Attachments:

v2-backbranch-REL_14-19.patch.txttext/plain; charset=US-ASCII; name=v2-backbranch-REL_14-19.patch.txtDownload+6-3
0002-psql-Add-test-for-unterminated-prompt-escapes.patch.txttext/plain; charset=US-ASCII; name=0002-psql-Add-test-for-unterminated-prompt-escapes.patch.txtDownload+50-1