On 3/12/17, Nicolas George <geo...@nsup.org> wrote: > Le duodi 22 ventose, an CCXXV, Paul B Mahol a ecrit : >> Because conflicting entries have not been added yet. Last time I >> compared it was different. > > Well, unlike some people on this mailing-list, I actually check my facts > before sending a mail. And I repeat, I did not see any conflict.
https://en.wikipedia.org/wiki/X11_color_names#Clashes_between_web_and_X11_colors_in_the_CSS_color_scheme > >> Also when Last time I tried it was soo slow that made 10k colors very >> slow to decode. > > Then make it faster, since obviously you are capable of it, but > duplicating it is unacceptable. Please, give yourself a big break. > >> > convert is not a very good name. >> >> OK, what you propose? > > hex_char_to_number(), for example. OK > >> >> + for (j = 0; reject && reject[j]; j++) { >> >> + if (string[i] == reject[j]) >> >> + break; >> > Use strchr(). >> That is slower. > > I very much doubt that. > >> >> +static uint32_t hexstring_to_rgba(const char *p, int len) >> > This is a misnomer. >> What it should be instead? > > Probably "color_string_to_rgba()". OK > >> >> + strncpy(color_name, p, len); >> >> + color_name[len] = '\0'; >> > This is completely wrong. >> What should it be instead? > > It should check len against sizeof(color_name), obviously. Could you not > find it yourself? The magic number in the size of the array should have > been a dead giveaway. OK > >> Better not return error and instead display what is already decoded. > > I strongly disagree. I absolutely disagree with you on this one. > >> > Also, you forgot to parse colors in standard X11 scheme >> > "rgb:RRRR/GGGG/BBBB". >> Are there such files? > > Of course. I yet have to find such files in wild. > >> > If I read this correctly, you are skipping random characters until a >> > quote is found. This is not how a robust parser should be written. >> Come on. > > Are you trying to communicate? I will just ignore this one. > >> >> + if (sscanf(ptr, "\"%u %u %u %u\",", >> >> + &avctx->width, &avctx->height, &ncolors, &cpp) != 4) { >> > This is not properly checking the final quote and comma. >> Really? > > Yes, really. Check the man page of sscanf() if you do not remember how > it works. I will just ignore this one. > >> >> + size = 1; >> >> + j = 1; >> >> + for (i = 0; i < cpp; i++) { >> >> + size += j*94; >> >> + j *= 95; >> >> + } >> >> + size *= 4; >> > >> > This is a DoS waiting to happen. >> Come on. I fuzzed this with afl-fuzzer up to 25 cycles. > > You should have given 25 seconds of thought instead. An attacker has > only to make cpp just large enough to eat all memory and give a few > colors to force the allocation of very sparse entries to actually access > it to make a DoS. I will just ignore this one. > >> >> + if (size < 0) { >> > This is deliberately invoking an undefined behaviour. >> How? > > The arithmetic can not make size negative, only an integer overflow. > > Furthermore, if there are several integer overflows, size can come back > positive but smaller than what will be accessed later, which is really > really bad. > > Actually, I think you just pushed an exploitable security hole. I dont think so. But anyway will be "fixed". > >> >> + ptr += mod_strcspn(ptr, ",") + 1; >> > Same remark as above: skipping random contents. Same for other uses of >> > mod_strcspn(). >> It is not skipping random contents. > > Oh? Then pray tell me what part of the code detects an invalid file with > random text at between the quote and the comma? Better to be robust and show image instead on breaking on corrupted stuff like nonascii chars in comments. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
