Re: BUG #7811: strlen(NULL) cause psql crash

On 15.01.2013 16:18, 1584171677@qq.com wrote:
>        I give you a description about how to trigger this bug first=EF=BC=
=9A
>            (1) start the server with the command "postgres -D pgdata"
>            (2) start the client with the command "psql"
>            (3) close the server
>            (4) execute a query from the client "slect *from t; ". At th=
is
> time, the client detected that it lost the connection with the server.
>            (5) execute the following command from the client "\?", then=
 the
> client will crash.
>
>        I have found the reason which caused that.
>
>            (1) When the client execute "slect *from t; ", it execute th=
e
> function "ResetCancelConn()" at line 364 in src\bin\psql\common.c ,and =
the
> function set pset.db to NULL.
>            (2) When the client execute "\?", it execute the function fp=
rintf
> at line 254 in help.c. The value returned by PQdb(pset.db) is an argume=
nt of
>   fprintf, and at this time   PQdb returned NULL.
>            (3) This NULL was finally passed to strlen at line 779 in
> snprintf.c through several simple fuction calls, so psql crashed.

Thanks for the report and debugging!

>        I hava fixed the bug in the following way which may be not the b=
est:
>
>            (1) add a string named strofnull, and in the function "dopr"=
 in
> file src\port\snprintf.c
>                    char *strofnull=3D"(null)";
>
>            (2) add an if statment before calling fmtstr at about line 7=
20 in
> file src\port\snprintf.c
>
>                    if (strvalue=3D=3DNULL)
>           {
>                   strvalue=3Dstrofnull;
>           }

That'd change the behavior of all sprintf calls, not sure we want to go=20
there. Might not be a bad idea to avoid crashes if there are more bugs=20
like this, but one really should not pass NULL to sprintf to begin with.=20
I committed a local fix to help.c to print "none" as the database name=20
when not connected.

- Heikki