Skip to content

Commit d3583b5

Browse files
ryanmkurtzryanmkurtz
committed
Merge remote-tracking branch
'origin/GP-2070_James_more_variadic_analyzer_issues' (Closes #4154, Closes #4281)
2 parents b8425e8 + 35caabc commit d3583b5

File tree

4 files changed

+148
-34
lines changed

4 files changed

+148
-34
lines changed

‎Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/plugin/core/string/variadic/PcodeFunctionParser.java

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import java.util.*;
1919

20-
import ghidra.docking.settings.SettingsImpl;
2120
import ghidra.program.model.address.Address;
2221
import ghidra.program.model.data.*;
2322
import ghidra.program.model.listing.*;
@@ -33,13 +32,11 @@
3332
*/
3433
public class PcodeFunctionParser {
3534

36-
// All values within the range [32, 126] are ascii readable
37-
private static final int READABLE_ASCII_LOWER_BOUND = 32;
38-
private static final int READABLE_ASCII_UPPER_BOUND = 126;
3935
// How many bytes to read from a memory address when initial format
40-
// String cannot be found. This normally only happens for short format
41-
// Strings with lengths less than 5
42-
private static final int BUFFER_LENGTH = 20;
36+
// String cannot be found. This can happen when the format string
37+
// is too short or contains escape characters that thwart the
38+
// ASCII string analyzer
39+
private static final int NULL_TERMINATOR_PROBE = -1;
4340
private static final String CALL_INSTRUCTION = "CALL";
4441

4542
private Program program;
@@ -150,7 +147,7 @@ private void searchForHiddenFormatStrings(PcodeOpAST callOp,
150147
if ((type == null) || !(type instanceof Pointer)) {
151148
continue;
152149
}
153-
String formatStringCandidate = findFormatString(v.getAddress(), (Pointer) type);
150+
String formatStringCandidate = findNullTerminatedString(v.getAddress(), (Pointer) type);
154151
if (formatStringCandidate == null) {
155152
continue;
156153
}
@@ -175,7 +172,7 @@ private Address convertAddressToRamSpace(Address address) {
175172
* @param pointer Pointer "type" of string
176173
* @return format String
177174
*/
178-
private String findFormatString(Address address, Pointer pointer) {
175+
String findNullTerminatedString(Address address, Pointer pointer) {
179176

180177
if (!address.getAddressSpace().isConstantSpace()) {
181178
return null;
@@ -191,24 +188,14 @@ private String findFormatString(Address address, Pointer pointer) {
191188
//StringDataInstace.getStringDataInstance checks that charType is appropriate
192189
//and returns StringDataInstace.NULL_INSTANCE if not
193190
StringDataInstance stringDataInstance = StringDataInstance.getStringDataInstance(charType,
194-
memoryBuffer, SettingsImpl.NO_SETTINGS, BUFFER_LENGTH);
195-
String stringValue = stringDataInstance.getStringValue();
196-
if (stringValue == null) {
191+
memoryBuffer, charType.getDefaultSettings(), NULL_TERMINATOR_PROBE);
192+
int detectedLength = stringDataInstance.getStringLength();
193+
if (detectedLength == -1) {
197194
return null;
198195
}
199-
200-
String formatStringCandidate = "";
201-
for (int i = 0; i < stringValue.length(); i++) {
202-
if (!isAsciiReadable(stringValue.charAt(i))) {
203-
break;
204-
}
205-
formatStringCandidate += stringValue.charAt(i);
206-
}
207-
return formatStringCandidate;
196+
stringDataInstance = new StringDataInstance(charType, charType.getDefaultSettings(),
197+
memoryBuffer, detectedLength, true);
198+
return stringDataInstance.getStringValue();
208199
}
209200

210-
private boolean isAsciiReadable(char c) {
211-
212-
return c >= READABLE_ASCII_LOWER_BOUND && c <= READABLE_ASCII_UPPER_BOUND;
213-
}
214201
}

‎Ghidra/Features/DecompilerDependent/src/test.slow/java/ghidra/app/plugin/core/string/variadic/FormatStringFinderTest.java

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/* ###
2+
* IP: GHIDRA
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package ghidra.app.plugin.core.string.variadic;
17+
18+
import static org.junit.Assert.*;
19+
20+
import java.io.UnsupportedEncodingException;
21+
import java.nio.charset.StandardCharsets;
22+
23+
import org.junit.Test;
24+
25+
import ghidra.program.database.ProgramBuilder;
26+
import ghidra.program.model.address.Address;
27+
import ghidra.program.model.address.AddressFactory;
28+
import ghidra.program.model.data.*;
29+
import ghidra.program.model.listing.Program;
30+
import ghidra.test.AbstractProgramBasedTest;
31+
import ghidra.util.Msg;
32+
33+
public class FormatStringFinderTest extends AbstractProgramBasedTest {
34+
35+
private static final int BASE_ADDRESS = 0x10000;
36+
private static final int LENGTH = 0x1000;
37+
38+
private static final String SIMPLE_FORMAT_STRING = "length: %d";
39+
private static final byte[] SIMPLE_FORMAT_BYTES =
40+
SIMPLE_FORMAT_STRING.getBytes(StandardCharsets.US_ASCII);
41+
42+
private static final int SIMPLE_NON_FORMAT_START = 0x10010;
43+
private static final String SIMPLE_NON_FORMAT_STRING = "hello";
44+
private static final byte[] SIMPLE_NON_FORMAT_BYTES =
45+
SIMPLE_NON_FORMAT_STRING.getBytes(StandardCharsets.US_ASCII);
46+
47+
private static final int WIDE_CHAR_STRING_START = 0x10020;
48+
private static final String WCHAR_EXAMPLE = "%s";
49+
private boolean wcharEncoded = false;
50+
51+
private static final int SHORT_FORMAT_STRING_START = 0x10030;
52+
private static final int SHORT_NEAR_END_START = BASE_ADDRESS + LENGTH - 4;
53+
private static final String SHORT_FORMAT_STRING = "%d";
54+
private static final byte[] SHORT_FORMAT_STRING_BYTES =
55+
SHORT_FORMAT_STRING.getBytes(StandardCharsets.US_ASCII);
56+
57+
private static final int ANSI_COLOR_CODE_START = 0x10040;
58+
private static final String ANSI_COLOR_CODE_STRING = "reproduces error : %s!\n\u001b[0m";
59+
private static final byte[] ANSI_COLOR_CODE_BYTES =
60+
ANSI_COLOR_CODE_STRING.getBytes(StandardCharsets.US_ASCII);
61+
62+
@Test
63+
public void testNullTerminatedStringFinder() throws Exception {
64+
initialize();
65+
PcodeFunctionParser parser = new PcodeFunctionParser(program);
66+
AddressFactory addrFactory = program.getAddressFactory();
67+
68+
Pointer charPointer = program.getDataTypeManager().getPointer(new CharDataType());
69+
Pointer widePointer = program.getDataTypeManager()
70+
.getPointer(new WideCharDataType(program.getDataTypeManager()));
71+
72+
Address base = addrFactory.getConstantAddress(BASE_ADDRESS);
73+
String simpleFormat = parser.findNullTerminatedString(base, charPointer);
74+
assertEquals(SIMPLE_FORMAT_STRING, simpleFormat);
75+
76+
Address simpleNonFormatAddr = addrFactory.getConstantAddress(SIMPLE_NON_FORMAT_START);
77+
String simpleNonFormat = parser.findNullTerminatedString(simpleNonFormatAddr, charPointer);
78+
assertEquals(SIMPLE_NON_FORMAT_STRING, simpleNonFormat);
79+
80+
if (wcharEncoded) {
81+
Address wideCharStringAddr = addrFactory.getConstantAddress(WIDE_CHAR_STRING_START);
82+
String wide = parser.findNullTerminatedString(wideCharStringAddr, widePointer);
83+
assertEquals(WCHAR_EXAMPLE, wide);
84+
}
85+
86+
Address shortAddr = addrFactory.getConstantAddress(SHORT_FORMAT_STRING_START);
87+
String shortString = parser.findNullTerminatedString(shortAddr, charPointer);
88+
assertEquals(SHORT_FORMAT_STRING, shortString);
89+
90+
Address shortNearEndAddr = addrFactory.getConstantAddress(SHORT_NEAR_END_START);
91+
shortString = parser.findNullTerminatedString(shortNearEndAddr, charPointer);
92+
assertEquals(SHORT_FORMAT_STRING, shortString);
93+
94+
Address ansiColorAddr = addrFactory.getConstantAddress(ANSI_COLOR_CODE_START);
95+
String ansiColorString = parser.findNullTerminatedString(ansiColorAddr, charPointer);
96+
assertEquals(ANSI_COLOR_CODE_STRING, ansiColorString);
97+
98+
}
99+
100+
@Override
101+
protected Program getProgram() {
102+
ProgramBuilder builder = null;
103+
try {
104+
builder = new ProgramBuilder("test", ProgramBuilder._X64, "gcc", null);
105+
builder.createMemory("test", Integer.toHexString(BASE_ADDRESS), LENGTH, "test",
106+
(byte) 0x0);
107+
builder.setBytes(Integer.toHexString(BASE_ADDRESS), SIMPLE_FORMAT_BYTES);
108+
builder.setBytes(Integer.toHexString(SIMPLE_NON_FORMAT_START), SIMPLE_NON_FORMAT_BYTES);
109+
try {
110+
builder.setBytes(Integer.toHexString(WIDE_CHAR_STRING_START),
111+
WCHAR_EXAMPLE.getBytes("UTF-32LE"));
112+
wcharEncoded = true;
113+
}
114+
catch (UnsupportedEncodingException e) {
115+
Msg.warn(this, e.getMessage());
116+
}
117+
builder.setBytes(Integer.toHexString(SHORT_FORMAT_STRING_START),
118+
SHORT_FORMAT_STRING_BYTES);
119+
builder.setBytes(Integer.toHexString(SHORT_NEAR_END_START), SHORT_FORMAT_STRING_BYTES);
120+
builder.setBytes(Integer.toHexString(ANSI_COLOR_CODE_START), ANSI_COLOR_CODE_BYTES);
121+
}
122+
catch (Exception e) {
123+
fail("Exception creating testing program: " + e.getMessage());
124+
}
125+
126+
return builder.getProgram();
127+
}
128+
129+
}

‎Ghidra/Features/DecompilerDependent/src/test/java/ghidra/app/plugin/core/string/variadic/FormatStringParserTest.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,8 @@ public void testComplexFormatString() {
107107
{ program.getDataTypeManager().getPointer(new CharDataType()), new LongDataType() };
108108
runFormatTest("#thisisatest%+-4.12s%#.1lin\nd2", expectedTypes2, true);
109109

110-
DataType[] expectedTypes3 =
111-
{ new PointerDataType(DataType.VOID), new LongDoubleDataType(),
112-
new UnsignedCharDataType() };
110+
DataType[] expectedTypes3 = { new PointerDataType(DataType.VOID), new LongDoubleDataType(),
111+
new UnsignedCharDataType() };
113112
runFormatTest("%01.3pp%%%#1.2Lg%%%%%hhXxn2", expectedTypes3, true);
114113

115114
DataType[] expectedTypes4 = { new IntegerDataType(), new IntegerDataType(),
@@ -124,10 +123,10 @@ public void testComplexFormatString() {
124123

125124
}
126125

127-
// Tests format strings that use astericks to add another int
126+
// Tests format strings that use asterisks to add another int
128127
// argument to determine field width or precision
129128
@Test
130-
public void testAsterickFormatString() {
129+
public void testAsteriskFormatString() {
131130
DataType[] expectedTypes1 = { new IntegerDataType(), new IntegerDataType() };
132131
runFormatTest("%*d", expectedTypes1, true);
133132

@@ -158,9 +157,8 @@ public void testLengthModifierFormatString() {
158157
DataType[] expectedTypes3 = { new UnsignedShortDataType(), new UnsignedCharDataType() };
159158
runFormatTest("%hx %hhu", expectedTypes3, true);
160159

161-
DataType[] expectedTypes4 =
162-
{ new UnsignedLongDataType(), new LongLongDataType(), new UnsignedLongLongDataType(),
163-
new PointerDataType(LongLongDataType.dataType) };
160+
DataType[] expectedTypes4 = { new UnsignedLongDataType(), new LongLongDataType(),
161+
new UnsignedLongLongDataType(), new PointerDataType(LongLongDataType.dataType) };
164162
runFormatTest("%lX %lld %llx %lln", expectedTypes4, true);
165163

166164
DataType[] expectedTypes5 =

‎Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/StringDataInstance.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ public int getDataLength() {
426426
* contains our n-bit character which will be tested for null-ness. (not the endian'ness of the
427427
* character set name - ie. "UTF-16BE")
428428
*
429-
* @return length of the string (NOT including null term if null term probe), in bytes, or -1 if
429+
* @return length of the string (INCLUDING null term if null term probe), in bytes, or -1 if
430430
* no terminator found.
431431
*/
432432
public int getStringLength() {

0 commit comments

Comments
 (0)