[x264-devel] Integer overflow in mb_analyse_intra

[Jeremy Dorfman]

While running x264 under Clang's UndefinedBehaviorSanitizer, I found a few
videos and settings that trigger an integer overflow in mb_analyse_intra.
The following line can occasionally try to take satd[i_pred_mode] below
INT_MIN (presently encoder/analyse.c:907):
    satd[i_pred_mode] -= 3 * lambda;

I've locally worked around this by using an int64_t and clamping the result
back to an int32_t, however I'd imagine that's not desirable for 32-bit
targets. That said, I've appended the patch. If this looks good, can
someone merge it? If not, can someone help me fix this in a more
appropriate way?

Thanks,
-Jeremy

diff --git a/encoder/analyse.c b/encoder/analyse.c
index fb241a7d..564fe73a 100644
--- a/encoder/analyse.c
+++ b/encoder/analyse.c
@@ -904,7 +904,8 @@ static void mb_analyse_intra( x264_t *h,
x264_mb_analysis_t *a, int i_satd_inter
                     ALIGNED_ARRAY_16( int32_t, satd,[9] );
                     h->pixf.intra_mbcmp_x3_4x4( p_src_by, p_dst_by, satd );
                     int favor_vertical = satd[I_PRED_4x4_H] >
satd[I_PRED_4x4_V];
-                    satd[i_pred_mode] -= 3 * lambda;
+                    int64_t new_satd = (int64_t)satd[i_pred_mode] - 3 *
(int64_t)lambda;
+                    satd[i_pred_mode] = X264_MAX(INT_MIN, new_satd);
                     i_best = satd[I_PRED_4x4_DC]; a->i_predict4x4[idx] =
I_PRED_4x4_DC;
                     COPY2_IF_LT( i_best, satd[I_PRED_4x4_H],
a->i_predict4x4[idx], I_PRED_4x4_H );
                     COPY2_IF_LT( i_best, satd[I_PRED_4x4_V],
a->i_predict4x4[idx], I_PRED_4x4_V );
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x264-devel/attachments/20190306/c3afce0b/attachment.html>