Skip to content

Commit 6c5868c

Browse files
nickrolfenickrolfe
committed
Ruby: use NumberUtils in parseInteger
And make parse{Binary,Octal,Hex}Int hold only for values in the range 0 to 2^31-1 (incl.)
1 parent 6bd9616 commit 6c5868c

File tree

4 files changed

+87
-78
lines changed

4 files changed

+87
-78
lines changed

‎ruby/ql/lib/codeql/NumberUtils.qll‎

Lines changed: 71 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,37 @@
33
* representations.
44
*/
55

6+
/**
7+
* Gets the integer value of `binary` when interpreted as binary. `binary` must
8+
* contain only the digits 0 and 1. For values greater than
9+
* 01111111111111111111111111111111 (2^31-1, the maximum value that `int` can
10+
* represent), there is no result.
11+
*
12+
* ```
13+
* "0" => 0
14+
* "01" => 1
15+
* "1010101" => 85
16+
* ```
17+
*/
18+
bindingset[binary]
19+
int parseBinaryInt(string binary) {
20+
exists(string stripped | stripped = stripLeadingZeros(binary) |
21+
stripped.length() <= 31 and
22+
result >= 0 and
23+
result =
24+
sum(int index, string c, int digit |
25+
c = stripped.charAt(index) and
26+
digit = "01".indexOf(c)
27+
|
28+
twoToThe(stripped.length() - 1 - index) * digit
29+
)
30+
)
31+
}
32+
633
/**
734
* Gets the integer value of `hex` when interpreted as hex. `hex` must be a
8-
* valid hexadecimal string and, for integer-wrapping reasons, no longer than 6
9-
* digits.
35+
* valid hexadecimal string. For values greater than 7FFFFFFF (2^31-1, the
36+
* maximum value that `int` can represent), there is no result.
1037
*
1138
* ```
1239
* "0" => 0
@@ -16,19 +43,23 @@
1643
*/
1744
bindingset[hex]
1845
int parseHexInt(string hex) {
19-
hex.length() <= 6 and
20-
result =
21-
sum(int index, string c |
22-
c = hex.charAt(index)
23-
|
24-
sixteenToThe(hex.length() - 1 - index) * toHex(c)
25-
)
46+
exists(string stripped | stripped = stripLeadingZeros(hex) |
47+
stripped.length() <= 8 and
48+
result >= 0 and
49+
result =
50+
sum(int index, string c |
51+
c = stripped.charAt(index)
52+
|
53+
sixteenToThe(stripped.length() - 1 - index) * toHex(c)
54+
)
55+
)
2656
}
2757

2858
/**
2959
* Gets the integer value of `octal` when interpreted as octal. `octal` must be
30-
* a valid octal string and, for integer-wrapping reasons, no longer than 10
31-
* digits.
60+
* a valid octal string containing only the digits 0-7. For values greater than
61+
* 17777777777 (2^31-1, the maximum value that `int` can represent), there is no
62+
* result.
3263
*
3364
* ```
3465
* "0" => 0
@@ -38,13 +69,17 @@ int parseHexInt(string hex) {
3869
*/
3970
bindingset[octal]
4071
int parseOctalInt(string octal) {
41-
octal.length() <= 10 and
42-
result =
43-
sum(int index, string c |
44-
c = octal.charAt(index)
45-
|
46-
eightToThe(octal.length() - 1 - index) * toOctal(c)
47-
)
72+
exists(string stripped | stripped = stripLeadingZeros(octal) |
73+
stripped.length() <= 11 and
74+
result >= 0 and
75+
result =
76+
sum(int index, string c, int digit |
77+
c = stripped.charAt(index) and
78+
digit = "01234567".indexOf(c)
79+
|
80+
eightToThe(stripped.length() - 1 - index) * digit
81+
)
82+
)
4883
}
4984

5085
/** Gets the integer value of the `hex` char. */
@@ -65,33 +100,30 @@ private int toHex(string hex) {
65100
result = 15 and hex = ["f", "F"]
66101
}
67102

68-
/** Gets the integer value of the `octal` char. */
69-
private int toOctal(string octal) {
70-
octal = "0" and result = 0
71-
or
72-
octal = "1" and result = 1
73-
or
74-
octal = "2" and result = 2
75-
or
76-
octal = "3" and result = 3
77-
or
78-
octal = "4" and result = 4
79-
or
80-
octal = "5" and result = 5
81-
or
82-
octal = "6" and result = 6
83-
or
84-
octal = "7" and result = 7
85-
}
86-
87-
/** Gets the value of 16 to the power of `n`. */
103+
/**
104+
* Gets the value of 16 to the power of `n`. Holds only for `n` in the range
105+
* 0..7 (inclusive).
106+
*/
88107
int sixteenToThe(int n) {
89108
// 16**7 is the largest power of 16 that fits in an int.
90109
n in [0 .. 7] and result = 1.bitShiftLeft(4 * n)
91110
}
92111

93-
/** Gets the value of 8 to the power of `n`. */
112+
/**
113+
* Gets the value of 8 to the power of `n`. Holds only for `n` in the range
114+
* 0..10 (inclusive).
115+
*/
94116
int eightToThe(int n) {
95117
// 8**10 is the largest power of 8 that fits in an int.
96118
n in [0 .. 10] and result = 1.bitShiftLeft(3 * n)
97119
}
120+
121+
/**
122+
* Gets the value of 2 to the power of `n`. Holds only for `n` in the range
123+
* 0..30 (inclusive).
124+
*/
125+
int twoToThe(int n) { n in [0 .. 30] and result = 1.bitShiftLeft(n) }
126+
127+
/** Gets `s` with any leading "0" characters removed. */
128+
bindingset[s]
129+
private string stripLeadingZeros(string s) { result = s.regexpCapture("0*(.*)", 1) }

‎ruby/ql/lib/codeql/ruby/ast/internal/Literal.qll‎

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,37 +10,16 @@ int parseInteger(Ruby::Integer i) {
1010
s.charAt(0) != "0" and
1111
result = s.toInt()
1212
or
13-
exists(string str, string values, int shift |
14-
s.matches("0b%") and
15-
values = "01" and
16-
str = s.suffix(2) and
17-
shift = 1
18-
or
19-
s.matches("0x%") and
20-
values = "0123456789abcdef" and
21-
str = s.suffix(2) and
22-
shift = 4
23-
or
24-
s.charAt(0) = "0" and
25-
not s.charAt(1) = ["b", "x", "o"] and
26-
values = "01234567" and
27-
str = s.suffix(1) and
28-
shift = 3
29-
or
30-
s.matches("0o%") and
31-
values = "01234567" and
32-
str = s.suffix(2) and
33-
shift = 3
34-
|
35-
result =
36-
sum(int index, string c, int v, int exp |
37-
c = str.charAt(index) and
38-
v = values.indexOf(c.toLowerCase()) and
39-
exp = str.length() - index - 1
40-
|
41-
v.bitShiftLeft((str.length() - index - 1) * shift)
42-
)
43-
)
13+
s.matches("0b%") and result = parseBinaryInt(s.suffix(2))
14+
or
15+
s.matches("0x%") and result = parseHexInt(s.suffix(2))
16+
or
17+
s.charAt(0) = "0" and
18+
not s.charAt(1) = ["b", "x", "o"] and
19+
result = parseOctalInt(s.suffix(1))
20+
or
21+
s.matches("0o%") and
22+
result = parseOctalInt(s.suffix(2))
4423
)
4524
}
4625

‎ruby/ql/test/library-tests/ast/ValueText.expected‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,8 +454,6 @@
454454
| literals/literals.rb:12:1:12:1 | 0 | 0 |
455455
| literals/literals.rb:13:1:13:5 | 0d900 | 0 |
456456
| literals/literals.rb:16:1:16:6 | 0x1234 | 4660 |
457-
| literals/literals.rb:17:1:17:10 | 0xdeadbeef | -559038737 |
458-
| literals/literals.rb:18:1:18:11 | 0xF00D_face | -267519282 |
459457
| literals/literals.rb:21:1:21:4 | 0123 | 83 |
460458
| literals/literals.rb:22:1:22:5 | 0o234 | 156 |
461459
| literals/literals.rb:23:1:23:6 | 0O45_6 | 302 |

‎ruby/ql/test/library-tests/ast/literals/literals.expected‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ allLiterals
1010
| literals.rb:12:1:12:1 | 0 | IntegerLiteral | 0 |
1111
| literals.rb:13:1:13:5 | 0d900 | IntegerLiteral | 0 |
1212
| literals.rb:16:1:16:6 | 0x1234 | IntegerLiteral | 4660 |
13-
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | -559038737 |
14-
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | -267519282 |
13+
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | <none> |
14+
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | <none> |
1515
| literals.rb:21:1:21:4 | 0123 | IntegerLiteral | 83 |
1616
| literals.rb:22:1:22:5 | 0o234 | IntegerLiteral | 156 |
1717
| literals.rb:23:1:23:6 | 0O45_6 | IntegerLiteral | 302 |
@@ -765,8 +765,8 @@ numericLiterals
765765
| literals.rb:12:1:12:1 | 0 | IntegerLiteral | 0 |
766766
| literals.rb:13:1:13:5 | 0d900 | IntegerLiteral | 0 |
767767
| literals.rb:16:1:16:6 | 0x1234 | IntegerLiteral | 4660 |
768-
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | -559038737 |
769-
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | -267519282 |
768+
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | <none> |
769+
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | <none> |
770770
| literals.rb:21:1:21:4 | 0123 | IntegerLiteral | 83 |
771771
| literals.rb:22:1:22:5 | 0o234 | IntegerLiteral | 156 |
772772
| literals.rb:23:1:23:6 | 0O45_6 | IntegerLiteral | 302 |
@@ -836,8 +836,8 @@ integerLiterals
836836
| literals.rb:12:1:12:1 | 0 | IntegerLiteral | 0 |
837837
| literals.rb:13:1:13:5 | 0d900 | IntegerLiteral | 0 |
838838
| literals.rb:16:1:16:6 | 0x1234 | IntegerLiteral | 4660 |
839-
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | -559038737 |
840-
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | -267519282 |
839+
| literals.rb:17:1:17:10 | 0xdeadbeef | IntegerLiteral | <none> |
840+
| literals.rb:18:1:18:11 | 0xF00D_face | IntegerLiteral | <none> |
841841
| literals.rb:21:1:21:4 | 0123 | IntegerLiteral | 83 |
842842
| literals.rb:22:1:22:5 | 0o234 | IntegerLiteral | 156 |
843843
| literals.rb:23:1:23:6 | 0O45_6 | IntegerLiteral | 302 |

0 commit comments

Comments
 (0)