You're receiving this bug because the package in Summary has produced _FORTIFY_SOURCE related warnings indicating the presence of a sure overflow in a static buffer. Even though this is not always an indication of a security problem it might even be. So please check this out ASAP. By the way, _FORTIFY_SOURCE is disabled when you disable optimisation, so don't try finding out the cause using -O0. Thanks, Your friendly neighborhood tinderboxer
Created attachment 248566 [details] Build log (compressed)
Created attachment 248877 [details, diff] Patch to avidemux-2.5.3-r2.ebuild to fix fgets overflow This patch adds a sed statement to rewrite the fgets calls to use sizeof to compute the available length, rather than using a hardcoded magic number. There are other issues that should probably be addressed separately, such as the use of a bundled ffmpeg snapshot of r22831.
Kevin, thanks for the patch. I must admin that I am not that good in sed/regexp I need to review this patch carefully to understand exactly what it does. I hope to apply your patch this weekend Thank you!
(In reply to comment #3) > I must admin that I am not that good in sed/regexp I need to review this patch carefully to understand exactly what it does. I am happy to explain how it works, then. :) + -e 's/\(fgets\s*(\([[:alpha:]]\+\),\)\s*\([[:digit:]]\+\|[A-Z_]\+\)\(,\s*[a-z_]\+)\)/\1 sizeof(\2)\4/' \ One substitute, matching the regex: \(fgets\s*(\([[:alpha:]]\+\),\)\s*\([[:digit:]]\+\|[A-Z_]\+\)\(,\s*[a-z_]\+)\) and replacing it with the expression: \1 sizeof(\2)\4 Digging into the regex, we have four capturing expressions, some nested within others. The first capture is: \(fgets\s*(\([[:alpha:]]\+\),\) This captures a literal 'fgets', zero or more whitespace, a literal open parenthesis, the second capture (described below), and a literal comma. The second capture, embedded within the first, is: \([[:alpha:]]\+\) This captures a sequence of one or more characters in the character class alpha. This class is defined by POSIX. Generally, it contains only letters and is more portable than specifying [a-zA-Z], since some character sets (e.g. EBCDIC) do not have all the characters contiguous. It can also be clearer to read. See http://en.wikipedia.org/wiki/Regular_expression#POSIX_character_classes for a full list of POSIX character classes. Getting back to the regex, we resume at column 34, the second occurrence of \s*. This matches zero or more whitespace, and does not capture it. We then have the third capture: \([[:digit:]]\+\|[A-Z_]\+\) This matches one of two alternatives: '[[:digit:]]\+' or '[A-Z_]\+', as separated by '\|'. The first alternative is a sequence of one or more [:digit:], generally the set of decimal numbers. The second alternative is a sequence of one or more uppercase letters and underscores, in any order. (I could have used [:upper:] here instead of A-Z, but did not.) This is a capture expression because capturing ensured the proper binding of the alternative operator. Looking back, that may not have been necessary, but it does no harm. However, if the capturing markers are removed, that would renumber capture #4. Finally, we come to the last capture, which also concludes the regex: \(,\s*[a-z_]\+)\) This captures a comma, a sequence of zero or more whitespace, one or more lowercase letters and underscores, and a literal close parenthesis. If a line manages to match all those pieces, then the matched substring is replaced with the result of expanding \1 sizeof(\2)\4. This inserts the results of capture expression 1 ("fgets(<first argument>,"), literal "sizeof(", the result of capture expression 2 ("<first argument>"), and the result of capture expression 4 (", <third argument>)"). This has the effect of discarding the second argument to fgets and replacing it with a sizeof the first argument, but only when the second argument is a magic number or a capitalized value (which, by inspection of avidemux source, I determined were preprocessor defines that expand to magic numbers) and the first and third arguments are relatively simple expressions. For example, it does not match if pointers are involved, since pointer syntax would require use of a star or an arrow, neither of which would match the expressions.
(In reply to comment #4) Well, I've never seen such wizard use of regexp :). But in a case like this, I think it'd be better to make a patch so we can send it upstream.
Created attachment 249367 [details, diff] Patch file equivalent of attachment #248877 [details, diff] (In reply to comment #5) > (In reply to comment #4) > Well, I've never seen such wizard use of regexp :). But in a case like this, I > think it'd be better to make a patch so we can send it upstream. As you wish. :) The attached patch was created by applying the previously described sed and collecting the diff of the before and after versions of the file. Do you already have an account with upstream that you can use to relay the patch? It appears they route patches through a forum rather than a bug tracker.
(In reply to comment #6) > Do you already have an account with upstream that you can use to relay the > patch? It appears they route patches through a forum rather than a bug > tracker. > No, I don't have one. @Markos: do you? I don't know how this particular part of avidemux works, so even if this patch fixes the potential overflow, may be the code doesn't work as expected. @Kevin: Have you try to trigger this specific part of the code to see how this works? Probably posting the patch upstream will give us the answer.
(In reply to comment #7) > I don't know how this particular part of avidemux works, so even if this patch > fixes the potential overflow, may be the code doesn't work as expected. I think it will be fine. The fortification warning triggers when the code gives fgets permission to overflow the buffer, by way of passing too large a length to fgets. My change simply forces fgets to obey the actual allocated length. Any builds without my patch can overflow the buffer, but might crash or otherwise behave strangely. > @Kevin: Have you try to trigger this specific part of the code to see how this > works? No. I do not use avidemux, and only hit upon this bug report because I was seeking bugs in this class ("_FORTIFY_SOURCE indicates presence of overflow").
(In reply to comment #7) > (In reply to comment #6) > > Do you already have an account with upstream that you can use to relay the > > patch? It appears they route patches through a forum rather than a bug > > tracker. > > > > No, I don't have one. > @Markos: do you? > No I contact them using e-mail service I will send it upstream and see what happens :) Thanks Kevin
The patch was merged upstream. I will apply it to the ebuild later today
Patch applied. Thanks Kevin
