* [bug report] crypto: aria - Implement ARIA symmetric cipher algorithm
@ 2022-07-19 8:14 Dan Carpenter
2022-07-19 15:27 ` Taehee Yoo
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-07-19 8:14 UTC (permalink / raw)
To: ap420073; +Cc: linux-crypto
Hello Taehee Yoo,
The patch e4e712bbbd6d: "crypto: aria - Implement ARIA symmetric
cipher algorithm" from Jul 4, 2022, leads to the following Smatch
static checker warning:
crypto/aria.c:69 aria_set_encrypt_key() error: buffer overflow 'ck' 4 <= 4
crypto/aria.c:70 aria_set_encrypt_key() error: buffer overflow 'ck' 4 <= 5
crypto/aria.c:71 aria_set_encrypt_key() error: buffer overflow 'ck' 4 <= 6
crypto/aria.c:72 aria_set_encrypt_key() error: buffer overflow 'ck' 4 <= 7
crypto/aria.c:86 aria_set_encrypt_key() error: buffer overflow 'ck' 4 <= 8
crypto/aria.c:87 aria_set_encrypt_key() error: buffer overflow 'ck' 4 <= 9
crypto/aria.c:88 aria_set_encrypt_key() error: buffer overflow 'ck' 4 <= 10
crypto/aria.c:89 aria_set_encrypt_key() error: buffer overflow 'ck' 4 <= 11
crypto/aria.c
19 static void aria_set_encrypt_key(struct aria_ctx *ctx, const u8 *in_key,
20 unsigned int key_len)
21 {
22 const __be32 *key = (const __be32 *)in_key;
23 u32 w0[4], w1[4], w2[4], w3[4];
24 u32 reg0, reg1, reg2, reg3;
25 const u32 *ck;
26 int rkidx = 0;
27
28 ck = &key_rc[(key_len - 16) / 8][0];
key_rc is declared like this:
static const u32 key_rc[5][4] = {
29
30 w0[0] = be32_to_cpu(key[0]);
31 w0[1] = be32_to_cpu(key[1]);
32 w0[2] = be32_to_cpu(key[2]);
33 w0[3] = be32_to_cpu(key[3]);
34
35 reg0 = w0[0] ^ ck[0];
36 reg1 = w0[1] ^ ck[1];
37 reg2 = w0[2] ^ ck[2];
38 reg3 = w0[3] ^ ck[3];
39
40 aria_subst_diff_odd(®0, ®1, ®2, ®3);
41
42 if (key_len > 16) {
43 w1[0] = be32_to_cpu(key[4]);
44 w1[1] = be32_to_cpu(key[5]);
45 if (key_len > 24) {
46 w1[2] = be32_to_cpu(key[6]);
47 w1[3] = be32_to_cpu(key[7]);
48 } else {
49 w1[2] = 0;
50 w1[3] = 0;
51 }
52 } else {
53 w1[0] = 0;
54 w1[1] = 0;
55 w1[2] = 0;
56 w1[3] = 0;
57 }
58
59 w1[0] ^= reg0;
60 w1[1] ^= reg1;
61 w1[2] ^= reg2;
62 w1[3] ^= reg3;
63
64 reg0 = w1[0];
65 reg1 = w1[1];
66 reg2 = w1[2];
67 reg3 = w1[3];
68
--> 69 reg0 ^= ck[4];
So 4 and above is out of bounds.
70 reg1 ^= ck[5];
71 reg2 ^= ck[6];
72 reg3 ^= ck[7];
73
74 aria_subst_diff_even(®0, ®1, ®2, ®3);
75
76 reg0 ^= w0[0];
77 reg1 ^= w0[1];
78 reg2 ^= w0[2];
79 reg3 ^= w0[3];
80
81 w2[0] = reg0;
82 w2[1] = reg1;
83 w2[2] = reg2;
84 w2[3] = reg3;
85
86 reg0 ^= ck[8];
87 reg1 ^= ck[9];
88 reg2 ^= ck[10];
89 reg3 ^= ck[11];
90
91 aria_subst_diff_odd(®0, ®1, ®2, ®3);
92
93 w3[0] = reg0 ^ w1[0];
94 w3[1] = reg1 ^ w1[1];
95 w3[2] = reg2 ^ w1[2];
96 w3[3] = reg3 ^ w1[3];
97
98 aria_gsrk(ctx->enc_key[rkidx], w0, w1, 19);
99 rkidx++;
100 aria_gsrk(ctx->enc_key[rkidx], w1, w2, 19);
101 rkidx++;
102 aria_gsrk(ctx->enc_key[rkidx], w2, w3, 19);
103 rkidx++;
104 aria_gsrk(ctx->enc_key[rkidx], w3, w0, 19);
105
106 rkidx++;
107 aria_gsrk(ctx->enc_key[rkidx], w0, w1, 31);
108 rkidx++;
109 aria_gsrk(ctx->enc_key[rkidx], w1, w2, 31);
110 rkidx++;
111 aria_gsrk(ctx->enc_key[rkidx], w2, w3, 31);
112 rkidx++;
113 aria_gsrk(ctx->enc_key[rkidx], w3, w0, 31);
114
115 rkidx++;
116 aria_gsrk(ctx->enc_key[rkidx], w0, w1, 67);
117 rkidx++;
118 aria_gsrk(ctx->enc_key[rkidx], w1, w2, 67);
119 rkidx++;
120 aria_gsrk(ctx->enc_key[rkidx], w2, w3, 67);
121 rkidx++;
122 aria_gsrk(ctx->enc_key[rkidx], w3, w0, 67);
123
124 rkidx++;
125 aria_gsrk(ctx->enc_key[rkidx], w0, w1, 97);
126 if (key_len > 16) {
127 rkidx++;
128 aria_gsrk(ctx->enc_key[rkidx], w1, w2, 97);
129 rkidx++;
130 aria_gsrk(ctx->enc_key[rkidx], w2, w3, 97);
131
132 if (key_len > 24) {
133 rkidx++;
134 aria_gsrk(ctx->enc_key[rkidx], w3, w0, 97);
135
136 rkidx++;
137 aria_gsrk(ctx->enc_key[rkidx], w0, w1, 109);
138 }
139 }
140 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] crypto: aria - Implement ARIA symmetric cipher algorithm
2022-07-19 8:14 [bug report] crypto: aria - Implement ARIA symmetric cipher algorithm Dan Carpenter
@ 2022-07-19 15:27 ` Taehee Yoo
0 siblings, 0 replies; 2+ messages in thread
From: Taehee Yoo @ 2022-07-19 15:27 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-crypto
Hi Dan,
Thank you so much for your report!
On 7/19/22 17:14, Dan Carpenter wrote:
> Hello Taehee Yoo,
>
> The patch e4e712bbbd6d: "crypto: aria - Implement ARIA symmetric
> cipher algorithm" from Jul 4, 2022, leads to the following Smatch
> static checker warning:
>
> crypto/aria.c:69 aria_set_encrypt_key() error: buffer overflow 'ck' 4
<= 4
> crypto/aria.c:70 aria_set_encrypt_key() error: buffer overflow 'ck' 4
<= 5
> crypto/aria.c:71 aria_set_encrypt_key() error: buffer overflow 'ck' 4
<= 6
> crypto/aria.c:72 aria_set_encrypt_key() error: buffer overflow 'ck' 4
<= 7
> crypto/aria.c:86 aria_set_encrypt_key() error: buffer overflow 'ck' 4
<= 8
> crypto/aria.c:87 aria_set_encrypt_key() error: buffer overflow 'ck' 4
<= 9
> crypto/aria.c:88 aria_set_encrypt_key() error: buffer overflow 'ck' 4
<= 10
> crypto/aria.c:89 aria_set_encrypt_key() error: buffer overflow 'ck' 4
<= 11
>
> crypto/aria.c
> 19 static void aria_set_encrypt_key(struct aria_ctx *ctx, const
u8 *in_key,
> 20 unsigned int key_len)
> 21 {
> 22 const __be32 *key = (const __be32 *)in_key;
> 23 u32 w0[4], w1[4], w2[4], w3[4];
> 24 u32 reg0, reg1, reg2, reg3;
> 25 const u32 *ck;
> 26 int rkidx = 0;
> 27
> 28 ck = &key_rc[(key_len - 16) / 8][0];
>
> key_rc is declared like this:
>
> static const u32 key_rc[5][4] = {
>
> 29
> 30 w0[0] = be32_to_cpu(key[0]);
> 31 w0[1] = be32_to_cpu(key[1]);
> 32 w0[2] = be32_to_cpu(key[2]);
> 33 w0[3] = be32_to_cpu(key[3]);
> 34
> 35 reg0 = w0[0] ^ ck[0];
> 36 reg1 = w0[1] ^ ck[1];
> 37 reg2 = w0[2] ^ ck[2];
> 38 reg3 = w0[3] ^ ck[3];
> 39
> 40 aria_subst_diff_odd(®0, ®1, ®2, ®3);
> 41
> 42 if (key_len > 16) {
> 43 w1[0] = be32_to_cpu(key[4]);
> 44 w1[1] = be32_to_cpu(key[5]);
> 45 if (key_len > 24) {
> 46 w1[2] = be32_to_cpu(key[6]);
> 47 w1[3] = be32_to_cpu(key[7]);
> 48 } else {
> 49 w1[2] = 0;
> 50 w1[3] = 0;
> 51 }
> 52 } else {
> 53 w1[0] = 0;
> 54 w1[1] = 0;
> 55 w1[2] = 0;
> 56 w1[3] = 0;
> 57 }
> 58
> 59 w1[0] ^= reg0;
> 60 w1[1] ^= reg1;
> 61 w1[2] ^= reg2;
> 62 w1[3] ^= reg3;
> 63
> 64 reg0 = w1[0];
> 65 reg1 = w1[1];
> 66 reg2 = w1[2];
> 67 reg3 = w1[3];
> 68
> --> 69 reg0 ^= ck[4];
>
> So 4 and above is out of bounds.
>
> 70 reg1 ^= ck[5];
> 71 reg2 ^= ck[6];
> 72 reg3 ^= ck[7];
> 73
> 74 aria_subst_diff_even(®0, ®1, ®2, ®3);
> 75
> 76 reg0 ^= w0[0];
> 77 reg1 ^= w0[1];
> 78 reg2 ^= w0[2];
> 79 reg3 ^= w0[3];
> 80
> 81 w2[0] = reg0;
> 82 w2[1] = reg1;
> 83 w2[2] = reg2;
> 84 w2[3] = reg3;
> 85
> 86 reg0 ^= ck[8];
> 87 reg1 ^= ck[9];
> 88 reg2 ^= ck[10];
> 89 reg3 ^= ck[11];
> 90
> 91 aria_subst_diff_odd(®0, ®1, ®2, ®3);
> 92
> 93 w3[0] = reg0 ^ w1[0];
> 94 w3[1] = reg1 ^ w1[1];
> 95 w3[2] = reg2 ^ w1[2];
> 96 w3[3] = reg3 ^ w1[3];
> 97
> 98 aria_gsrk(ctx->enc_key[rkidx], w0, w1, 19);
> 99 rkidx++;
> 100 aria_gsrk(ctx->enc_key[rkidx], w1, w2, 19);
> 101 rkidx++;
> 102 aria_gsrk(ctx->enc_key[rkidx], w2, w3, 19);
> 103 rkidx++;
> 104 aria_gsrk(ctx->enc_key[rkidx], w3, w0, 19);
> 105
> 106 rkidx++;
> 107 aria_gsrk(ctx->enc_key[rkidx], w0, w1, 31);
> 108 rkidx++;
> 109 aria_gsrk(ctx->enc_key[rkidx], w1, w2, 31);
> 110 rkidx++;
> 111 aria_gsrk(ctx->enc_key[rkidx], w2, w3, 31);
> 112 rkidx++;
> 113 aria_gsrk(ctx->enc_key[rkidx], w3, w0, 31);
> 114
> 115 rkidx++;
> 116 aria_gsrk(ctx->enc_key[rkidx], w0, w1, 67);
> 117 rkidx++;
> 118 aria_gsrk(ctx->enc_key[rkidx], w1, w2, 67);
> 119 rkidx++;
> 120 aria_gsrk(ctx->enc_key[rkidx], w2, w3, 67);
> 121 rkidx++;
> 122 aria_gsrk(ctx->enc_key[rkidx], w3, w0, 67);
> 123
> 124 rkidx++;
> 125 aria_gsrk(ctx->enc_key[rkidx], w0, w1, 97);
> 126 if (key_len > 16) {
> 127 rkidx++;
> 128 aria_gsrk(ctx->enc_key[rkidx], w1, w2, 97);
> 129 rkidx++;
> 130 aria_gsrk(ctx->enc_key[rkidx], w2, w3, 97);
> 131
> 132 if (key_len > 24) {
> 133 rkidx++;
> 134 aria_gsrk(ctx->enc_key[rkidx], w3,
w0, 97);
> 135
> 136 rkidx++;
> 137 aria_gsrk(ctx->enc_key[rkidx], w0,
w1, 109);
> 138 }
> 139 }
> 140 }
>
> regards,
> dan carpenter
I think this is a false positive of smatch.
ck is a pointer and it points key_rc, which is a double array.
But ck is used as a single array.
So, I think smatch warns it although there are actually no out-of-bounds.
I just tested changing key_rc to a single array.
There are no smatch warnings.
So, I will send a patch to avoid this smatch warning.
Thank you so much,
Taehee Yoo
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-07-19 15:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-19 8:14 [bug report] crypto: aria - Implement ARIA symmetric cipher algorithm Dan Carpenter
2022-07-19 15:27 ` Taehee Yoo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.