You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@gmail.com> on 2004/11/30 16:11:02 UTC
Re: svn commit: r107007 - /apr/apr/trunk/CHANGES /apr/apr/trunk/include/apr_lib.h /apr/apr/trunk/passwd/apr_getpass.c
On 30 Nov 2004 14:41:33 -0000, trawick@apache.org <tr...@apache.org> wrote:
> apr_password_get(): Fix the check for buffer overflow.
> --- apr/apr/trunk/include/apr_lib.h (original)
> +++ apr/apr/trunk/include/apr_lib.h Tue Nov 30 06:41:31 2004
> @@ -168,6 +168,8 @@
> * @param prompt The prompt to display
> * @param pwbuf Buffer to store the password
> * @param bufsize The length of the password buffer.
> + * @remark If the password entered must be truncated to fit in
> + * the provided buffer, APR_ENAMETOOLONG will be returned.
> */
> APR_DECLARE(apr_status_t) apr_password_get(const char *prompt, char *pwbuf,
> apr_size_t *bufsize);
another disturbance: we force caller to go to the trouble to pass by
address, but we don't update the size on output to indicate either the
number of bytes stored or the number of bytes needed; shrug or "fix"?
Re: svn commit: r107007 - /apr/apr/trunk/CHANGES /apr/apr/trunk/include/apr_lib.h /apr/apr/trunk/passwd/apr_getpass.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Tue, 30 Nov 2004 10:26:31 -0500 (EST), Cliff Woolley
<jw...@virginia.edu> wrote:
> On Tue, 30 Nov 2004, Jeff Trawick wrote:
>
>
>
> > > * @param pwbuf Buffer to store the password
> > > * @param bufsize The length of the password buffer.
> > > + * @remark If the password entered must be truncated to fit in
> > > + * the provided buffer, APR_ENAMETOOLONG will be returned.
> > > */
> > > APR_DECLARE(apr_status_t) apr_password_get(const char *prompt, char *pwbuf,
> > > apr_size_t *bufsize);
> >
> > another disturbance: we force caller to go to the trouble to pass by
> > address, but we don't update the size on output to indicate either the
> > number of bytes stored or the number of bytes needed; shrug or "fix"?
>
> By number of bytes stored, I assume you mean bytes stored in the non-error
> case, since bytes stored in the error case would just be bufsize. Right?
bufsize - 1, since I don't think we'd count the '\0'
>
> If that's the case, then it's maybe worth fixing but I'm not sure I care.
> How many bytes would be needed seems to make somewhat more sense as being
> a useful value, but do we have any other functions that work that way?
agreed on that being more useful
.
apr_strftime() is a function which has "apr_size_t *" to report the
length of the returned string, but on overflow it seems to return 0.
(apr_strftime() defers to strftime() to do the dirty work, and
strftime() doc says
If the
total number of resulting characters including the terminat-
ing null character is more than maxsize, strftime() returns
0 and the contents of the array are indeterminate.
)
.
an ugly mess all around; getpass() itself can truncate the user's
input but not report to us that it truncated (try entering more than 8
chars on HP-UX), so we can't accurately report truncation unless we
modify our getpass() implementation to report that, and always use our
own instead of the system getpass()
punt
Re: svn commit: r107007 - /apr/apr/trunk/CHANGES /apr/apr/trunk/include/apr_lib.h
/apr/apr/trunk/passwd/apr_getpass.c
Posted by Cliff Woolley <jw...@virginia.edu>.
On Tue, 30 Nov 2004, Jeff Trawick wrote:
> > * @param pwbuf Buffer to store the password
> > * @param bufsize The length of the password buffer.
> > + * @remark If the password entered must be truncated to fit in
> > + * the provided buffer, APR_ENAMETOOLONG will be returned.
> > */
> > APR_DECLARE(apr_status_t) apr_password_get(const char *prompt, char *pwbuf,
> > apr_size_t *bufsize);
>
> another disturbance: we force caller to go to the trouble to pass by
> address, but we don't update the size on output to indicate either the
> number of bytes stored or the number of bytes needed; shrug or "fix"?
By number of bytes stored, I assume you mean bytes stored in the non-error
case, since bytes stored in the error case would just be bufsize. Right?
If that's the case, then it's maybe worth fixing but I'm not sure I care.
How many bytes would be needed seems to make somewhat more sense as being
a useful value, but do we have any other functions that work that way?
Re: svn commit: r107007 - /apr/apr/trunk/CHANGES /apr/apr/trunk/include/apr_lib.h /apr/apr/trunk/passwd/apr_getpass.c
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, 1 Dec 2004 12:04:14 +0000, Joe Orton <jo...@redhat.com> wrote:
> On Tue, Nov 30, 2004 at 10:11:02AM -0500, Jeff Trawick wrote:
>
>
> > On 30 Nov 2004 14:41:33 -0000, trawick@apache.org <tr...@apache.org> wrote:
> > > apr_password_get(): Fix the check for buffer overflow.
> > > --- apr/apr/trunk/include/apr_lib.h (original)
> > > +++ apr/apr/trunk/include/apr_lib.h Tue Nov 30 06:41:31 2004
> > > @@ -168,6 +168,8 @@
> > > * @param prompt The prompt to display
> > > * @param pwbuf Buffer to store the password
> > > * @param bufsize The length of the password buffer.
> > > + * @remark If the password entered must be truncated to fit in
> > > + * the provided buffer, APR_ENAMETOOLONG will be returned.
> > > */
> > > APR_DECLARE(apr_status_t) apr_password_get(const char *prompt, char *pwbuf,
> > > apr_size_t *bufsize);
> >
> > another disturbance: we force caller to go to the trouble to pass by
> > address, but we don't update the size on output to indicate either the
> > number of bytes stored or the number of bytes needed; shrug or "fix"?
>
> I think "shrug" - any callers might as well call strlen() than rely on a
> future version of the function which updates *bufsize. I've updated to
> the docco to fix that in stone, hope that's OK.
+1
Re: svn commit: r107007 - /apr/apr/trunk/CHANGES /apr/apr/trunk/include/apr_lib.h /apr/apr/trunk/passwd/apr_getpass.c
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Nov 30, 2004 at 10:11:02AM -0500, Jeff Trawick wrote:
> On 30 Nov 2004 14:41:33 -0000, trawick@apache.org <tr...@apache.org> wrote:
> > apr_password_get(): Fix the check for buffer overflow.
> > --- apr/apr/trunk/include/apr_lib.h (original)
> > +++ apr/apr/trunk/include/apr_lib.h Tue Nov 30 06:41:31 2004
> > @@ -168,6 +168,8 @@
> > * @param prompt The prompt to display
> > * @param pwbuf Buffer to store the password
> > * @param bufsize The length of the password buffer.
> > + * @remark If the password entered must be truncated to fit in
> > + * the provided buffer, APR_ENAMETOOLONG will be returned.
> > */
> > APR_DECLARE(apr_status_t) apr_password_get(const char *prompt, char *pwbuf,
> > apr_size_t *bufsize);
>
> another disturbance: we force caller to go to the trouble to pass by
> address, but we don't update the size on output to indicate either the
> number of bytes stored or the number of bytes needed; shrug or "fix"?
I think "shrug" - any callers might as well call strlen() than rely on a
future version of the function which updates *bufsize. I've updated to
the docco to fix that in stone, hope that's OK.
joe