Hello Chuck Lever,
Commit b68e4c5c3227 ("SUNRPC: Convert unwrap_integ_data() to use
xdr_stream") from Jan 2, 2023 (linux-next), leads to the following
Smatch static checker warning:
net/sunrpc/auth_gss/svcauth_gss.c:895 svcauth_gss_unwrap_integ()
warn: potential user controlled sizeof overflow 'offset + 4'
net/sunrpc/auth_gss/svcauth_gss.c
859 static noinline_for_stack int
860 svcauth_gss_unwrap_integ(struct svc_rqst *rqstp, u32 seq, struct gss_ctx *ctx)
861 {
862 struct gss_svc_data *gsd = rqstp->rq_auth_data;
863 struct xdr_stream *xdr = &rqstp->rq_arg_stream;
864 u32 len, offset, seq_num, maj_stat;
865 struct xdr_buf *buf = xdr->buf;
866 struct xdr_buf databody_integ;
867 struct xdr_netobj checksum;
868
869 /* Did we already verify the signature on the original pass through? */
870 if (rqstp->rq_deferred)
871 return 0;
872
873 if (xdr_stream_decode_u32(xdr, &len) < 0)
^^^^
874 goto unwrap_failed;
875 if (len & 3)
There used a if (len > buf->len) here but it was deleted.
876 goto unwrap_failed;
877 offset = xdr_stream_pos(xdr);
878 if (xdr_buf_subsegment(buf, &databody_integ, offset, len))
I don't see any bounds checking in here but I might have missed it
879 goto unwrap_failed;
880
881 /*
882 * The xdr_stream now points to the @seq_num field. The next
883 * XDR data item is the @arg field, which contains the clear
884 * text RPC program payload. The checksum, which follows the
885 * @arg field, is located and decoded without updating the
886 * xdr_stream.
887 */
888
889 offset += len;
^^^^^^^^^^^^^
This could be an integer overflow?
890 if (xdr_decode_word(buf, offset, &checksum.len))
891 goto unwrap_failed;
892 if (checksum.len > sizeof(gsd->gsd_scratch))
893 goto unwrap_failed;
894 checksum.data = gsd->gsd_scratch;
--> 895 if (read_bytes_from_xdr_buf(buf, offset + XDR_UNIT, checksum.data,
^^^^^^^^^^^^^^^^^
This integer overflow warning is only about foo + sizeof() to cut down on false
positives.
896 checksum.len))
897 goto unwrap_failed;
898
899 maj_stat = gss_verify_mic(ctx, &databody_integ, &checksum);
900 if (maj_stat != GSS_S_COMPLETE)
901 goto bad_mic;
902
903 /* The received seqno is protected by the checksum. */
904 if (xdr_stream_decode_u32(xdr, &seq_num) < 0)
905 goto unwrap_failed;
906 if (seq_num != seq)
907 goto bad_seqno;
908
909 xdr_truncate_decode(xdr, XDR_UNIT + checksum.len);
910 return 0;
911
912 unwrap_failed:
913 trace_rpcgss_svc_unwrap_failed(rqstp);
914 return -EINVAL;
915 bad_seqno:
916 trace_rpcgss_svc_seqno_bad(rqstp, seq, seq_num);
917 return -EINVAL;
918 bad_mic:
919 trace_rpcgss_svc_mic(rqstp, maj_stat);
920 return -EINVAL;
921 }
regards,
dan carpenter
