On 5/21/17, David Simmons <d...@smallscript.com> wrote:
> Location: "fossil\src\printf.c"
> Section(s): (line 385)
>      /* Limit the precision to prevent overflowing buf[] during
> conversion */
>      if( precision > (etBUFSIZE-40) && (infop->flags & FLAG_STRING)==0 ){
>        precision = etBUFSIZE-40;
>      }
>
> Issue: missing parens to ensure correct ordering of expressing. Lacking
> parens, it sets precision to "etBUFSIZE-40" ~ (500-40) => 460
> Impact: inefficient behavior, possible output errors.

What compiler are you using that thinks that the ">" operator has
higher precedence than the "-" operator?

And I don't quite understand what the second complaint (below) is
about.  Could you explain the problem again - perhaps with an example
that shows the failure?

>
> Location: "fossil\src\printf.c"
> Section(s): (line 200 ... line 645)
>      static int StrNLen32(const char *z, int N){
>      ...elided...
>        case etSTRINGID:
>        case etSTRING:
>        case etDYNSTRING: {
>          int limit = flag_alternateform ? va_arg(ap,int) : -1;
>          bufpt = va_arg(ap,char*);
>          if(bufpt == 0){
>            bufpt = "";
>          }else if( xtype==etDYNSTRING ){
>            zExtra = bufpt;
>          }else if( xtype==etSTRINGID ){
>            precision = hashDigits(flag_altform2);
>          }
>          // printf rules say: (fixes from DSIM)
>          //  "width" is a minimum for the output of this param
>          //  "precision" is a maximum or exact size of this param
>          if(precision >= 0 && xtype != etSTRINGID) {
>            limit = length = precision;
>            int uLen32 = StrNLen32(bufpt, limit);
>            // DSim: assert(uLen32 == length);
>          }
>          else {
>            if(limit < 0) {
>              if(precision > 0)
>                limit = precision;
>              else if(width)
>                limit = width;
>            }
>            else if(limit > precision) {
>              limit = precision;
>            }
>            else if(limit > width) {
>              limit = width;
>            }
>            // DSIM: This code has a bug where it assumes NULL '\0'
> terminated bufpt
>            // However, if *precision* is provided and > 0 and limit is
> -1 then
>            // precision defines maximum number of characters in bufpt.
> Without
>            // that rule, and bufpt not being '\0' null terminated, this code
>            // BLOWS UP randomly since bufpt memory can be random beyond
> bufpt[precision].
>            length = StrNLen32(bufpt, limit); // <== SOURCE OF BUG (GOES
> BOOM RARELY, BUT RANDOMLY!)
>          }
>          break;
>        }
>
> Issue: code does not insure reading past end of z, due to calling code
> in '%...s' miscalculation of default value for N.
> Impact: intermittent app-crashes
> _______________________________________________
> fossil-dev mailing list
> fossil-dev@mailinglists.sqlite.org
> http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/fossil-dev
>


-- 
D. Richard Hipp
d...@sqlite.org
_______________________________________________
fossil-dev mailing list
fossil-dev@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/fossil-dev

Reply via email to