extmod/modframebuf.c framebuf1_text() does not clear pixels
This section of the code looks wrong:
if (vline_data & 1) { // only draw if pixel set
if (0 <= y && y < self->height) { // clip y
uint byte_pos = x0 + self->stride * ((uint)y >> 3);
if (col == 0) {
// clear pixel
self->buf[byte_pos] &= ~(1 << (y & 7));
} else {
// set pixel
self->buf[byte_pos] |= 1 << (y & 7);
}
}
If the pixel is not set, it should surely clear down the pixel in the framebuf as we don't know its prior contents. Further, if we happen to be drawing with col==0 to a part of the framebuf which is already zero, then nothing will be drawn, because in that circumstance the code never sets a pixel.
I hope I'm right on this observation from reviewing the code: I haven't had the opportunity to try it since my SSD1306 was DOA.
framebuf.FrameBuffer: not secured against silly/wrong/malicious strides
Dear Micropython Developers!
framebuf.FrameBuffer is not secured against the misuse with unreasonable values of the stride parameter.
IMHO :
- stride < width is nonsense (or only useful for esoteric matters) and should be set to width
- strides larger than len(buf)/width are an attack vector to disclose information
So this can happen:
buffer = bytearray([0, 0, 0, 0, 0xff, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff])
for stride in range(100):
fb = framebuf.FrameBuffer(buffer, 2, 2, framebuf.MONO_HLSB, stride)
a = fb.pixel(1,1)
print(stride, a)
The function getpixel should only operate in the buffers first four bytes, thus only returning zero values.
But the bit pattern of the wider buffer is exposed.
>>> %Run -c $EDITOR_CONTENT
0 0
1 0
2 0
3 0
4 0
5 0
6 0
7 0
8 0
9 0
10 0
11 0
12 0
13 0
14 0
15 0
16 0
17 0
18 0
19 0
20 0
21 0
22 0
23 0
24 0
25 1
26 1
27 1
28 1
29 1
30 1
31 1
32 1
33 0
34 0
35 0
36 0
37 0
38 0
39 0
40 0
41 1
42 1
43 1
44 1
45 1
46 1
47 1
48 1
49 1
50 1
51 1
52 1
53 1
54 1
55 1
56 1
57 1
58 1
59 1
60 1
61 1
62 1
63 1
64 1
65 1
66 1
67 1
68 1
69 1
70 1
71 1
72 1
73 1
74 1
75 1
76 1
77 1
78 1
79 1
80 1
81 1
82 1
83 1
84 1
85 1
86 1
87 1
88 1
89 0
90 0
91 0
92 0
93 0
94 0
95 0
96 0
97 0
98 0
99 0
This allows a malicious code to read arbitrary RAM positions.
To prevent this there should be checks for dangerous stride values while creating a FrameBuffer instance.
I noticed this while bringing up my testing for the new FrameBuffer.get_rect() routine.
Also there is not a single test in the existing framebuf testing code concerning the stride parameter.
Sorry for counting peas.
Cheers,
Volker