Buffer size checking for scanf* functions
Adhemerval Zanella
adhemerval.zanella@linaro.org
Mon Jul 11 12:38:50 GMT 2022
> On 6 Jul 2022, at 12:04, Yair Lenga <yair.lenga@gmail.com> wrote:
>
> Thanks for elaborating .Agree that when possible '-m' should be used, but it's not always trivial .Lot of code is already written in such a way that those changes are less than trivial. Not to mention that each %m will require adding an strcpy (or equivalent) to copy the dynamically allocated strings into the fixed length storage usually defined in struct, etc.
>
> struct { ... } s ;
> char *sx = NULL;
> scanf("%ms %d", &sx, &s.i) ;
> strcpy_fix(s.output, sizeof(s.output), &sx) ; // Copy from *sx to s.output, up to the limit, free *sx, and set sx = NULL
Well either this or to use the a simpler solution like:
#define NAMELEN 32
struct { char name[NAMELEN]; } x;
#define STRFY(__n) XSTRFY(__n)
#define XSTRFY(__n) #__n
scanf ("%"STRFY(NAMELEN)"s", x.name);
>
> Your point about backward compatibility is also very valid - if possible new features should try to avoid collision with future improvement. The C standard is getting updated every 10 years (c99, c11, and the expected c23), I could not find any reason why the C standard committee chose to use '%m' instead of using the already established '%a' that existed for many years in glibc, and assign new meaning to the '%a'. I hope that those are exceptions that proves the rules.
>
> You raised a good point with the 'scanf_s' - the fact that they chose to modify the behavior of the '%s' . To my understanding scanf_s '%s' requires 2 arguments (char *, size_t), vs. scanf that will only expect the 'char *'. It would have been a much better solution to keep '%s' compatible. and introduce another formatting sequence for the dynamic fixed-length string.
>
> Going back to the question - what will be a good way to integrate the type safety provided by scanf_s, without creating problems. a few ideas that I have:
> * Use '%S' (upper S) into indicate that a pair (char *, size_t) is expected, OR
> * Use '%@s' ('@' can be any unused letter or special character e.g. '%!s', '%:s', ...). The logical choice should have been '*' - symmetry with printf("%*s"). Unfortunately, '*' is already used .. as "ignore assignment' flag.
> * Use '%n %s', when 'n' will indicate a size parameter will be provided, and will apply to the next '%s' or '%[', or even '%ms' - dynamic width limit, instead of static width limit.
>
> Personally, I prefer the second option '%@s', it matches the style of '*' for printf. Easy for existing developers to grasp. Interesting enough, it might be possible to implement the scanf_s as a wrapper around scanf, with some manipulation of the argument list.
I don’t have a strong preference, and although I think scanf is still a
bad interface [1] I think you might try to raise this on libc-alpha.
I think the ‘@‘ modifier would make more sense, since ideally it would
extend to wscanf familiar as well (and it already defines ’%S’).
[1] https://github.com/biojppm/rapidyaml/issues/40
More information about the Libc-help
mailing list