Skip to content

Commit a71c072

Browse files
wing328wing328
authored
Merge pull request #3224 from wing328/security_fix
[PHP] Better code injection handling for PHP API client
2 parents 133c3ab + 41636ae commit a71c072

File tree

95 files changed

+6473
-444
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

95 files changed

+6473
-444
lines changed

‎.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ samples/client/petstore/php/SwaggerClient-php/composer.lock
7575
samples/client/petstore/php/SwaggerClient-php/vendor/
7676
samples/client/petstore/silex/SwaggerServer/composer.lock
7777
samples/client/petstore/silex/SwaggerServer/venodr/
78+
**/vendor/
79+
**/composer.lock
7880

7981
# Perl
8082
samples/client/petstore/perl/deep_module_test/

‎bin/security/lumen-petstore-server.sh‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#!/bin/sh
2+
3+
SCRIPT="$0"
4+
5+
while [ -h "$SCRIPT" ] ; do
6+
ls=`ls -ld "$SCRIPT"`
7+
link=`expr "$ls" : '.*-> \(.*\)$'`
8+
if expr "$link" : '/.*' > /dev/null; then
9+
SCRIPT="$link"
10+
else
11+
SCRIPT=`dirname "$SCRIPT"`/"$link"
12+
fi
13+
done
14+
15+
if [ ! -d "${APP_DIR}" ]; then
16+
APP_DIR=`dirname "$SCRIPT"`/..
17+
APP_DIR=`cd "${APP_DIR}"; pwd`
18+
fi
19+
20+
executable="./modules/swagger-codegen-cli/target/swagger-codegen-cli.jar"
21+
22+
if [ ! -f "$executable" ]
23+
then
24+
mvn clean package
25+
fi
26+
27+
# if you've executed sbt assembly previously it will use that instead.
28+
export JAVA_OPTS="${JAVA_OPTS} -XX:MaxPermSize=256M -Xmx1024M -DloggerPath=conf/log4j.properties"
29+
ags="$@ generate -t modules/swagger-codegen/src/main/resources/lumen -i modules/swagger-codegen/src/test/resources/2_0/petstore-security-test.yaml -l lumen -o samples/server/petstore-security-test/lumen"
30+
31+
java $JAVA_OPTS -jar $executable $ags

‎bin/security/php-petstore.sh‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#!/bin/sh
2+
3+
SCRIPT="$0"
4+
5+
while [ -h "$SCRIPT" ] ; do
6+
ls=`ls -ld "$SCRIPT"`
7+
link=`expr "$ls" : '.*-> \(.*\)$'`
8+
if expr "$link" : '/.*' > /dev/null; then
9+
SCRIPT="$link"
10+
else
11+
SCRIPT=`dirname "$SCRIPT"`/"$link"
12+
fi
13+
done
14+
15+
if [ ! -d "${APP_DIR}" ]; then
16+
APP_DIR=`dirname "$SCRIPT"`/..
17+
APP_DIR=`cd "${APP_DIR}"; pwd`
18+
fi
19+
20+
executable="./modules/swagger-codegen-cli/target/swagger-codegen-cli.jar"
21+
22+
if [ ! -f "$executable" ]
23+
then
24+
mvn clean package
25+
fi
26+
27+
# if you've executed sbt assembly previously it will use that instead.
28+
export JAVA_OPTS="${JAVA_OPTS} -XX:MaxPermSize=256M -Xmx1024M -DloggerPath=conf/log4j.properties"
29+
ags="$@ generate -t modules/swagger-codegen/src/main/resources/php -i modules/swagger-codegen/src/test/resources/2_0/petstore-security-test.yaml -l php -o samples/client/petstore-security-test/php"
30+
31+
java $JAVA_OPTS -jar $executable $ags

‎bin/security/silex-petstore-server.sh‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#!/bin/sh
2+
3+
SCRIPT="$0"
4+
5+
while [ -h "$SCRIPT" ] ; do
6+
ls=`ls -ld "$SCRIPT"`
7+
link=`expr "$ls" : '.*-> \(.*\)$'`
8+
if expr "$link" : '/.*' > /dev/null; then
9+
SCRIPT="$link"
10+
else
11+
SCRIPT=`dirname "$SCRIPT"`/"$link"
12+
fi
13+
done
14+
15+
if [ ! -d "${APP_DIR}" ]; then
16+
APP_DIR=`dirname "$SCRIPT"`/..
17+
APP_DIR=`cd "${APP_DIR}"; pwd`
18+
fi
19+
20+
executable="./modules/swagger-codegen-cli/target/swagger-codegen-cli.jar"
21+
22+
if [ ! -f "$executable" ]
23+
then
24+
mvn clean package
25+
fi
26+
27+
# if you've executed sbt assembly previously it will use that instead.
28+
export JAVA_OPTS="${JAVA_OPTS} -XX:MaxPermSize=256M -Xmx1024M -DloggerPath=conf/log4j.properties"
29+
ags="$@ generate -t modules/swagger-codegen/src/main/resources/silex -i modules/swagger-codegen/src/test/resources/2_0/petstore-security-test.yaml -l silex-PHP -o samples/server/petstore-security-test/silex"
30+
31+
java $JAVA_OPTS -jar $executable $ags

‎bin/security/slim-petstore-server.sh‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#!/bin/sh
2+
3+
SCRIPT="$0"
4+
5+
while [ -h "$SCRIPT" ] ; do
6+
ls=`ls -ld "$SCRIPT"`
7+
link=`expr "$ls" : '.*-> \(.*\)$'`
8+
if expr "$link" : '/.*' > /dev/null; then
9+
SCRIPT="$link"
10+
else
11+
SCRIPT=`dirname "$SCRIPT"`/"$link"
12+
fi
13+
done
14+
15+
if [ ! -d "${APP_DIR}" ]; then
16+
APP_DIR=`dirname "$SCRIPT"`/..
17+
APP_DIR=`cd "${APP_DIR}"; pwd`
18+
fi
19+
20+
executable="./modules/swagger-codegen-cli/target/swagger-codegen-cli.jar"
21+
22+
if [ ! -f "$executable" ]
23+
then
24+
mvn clean package
25+
fi
26+
27+
# if you've executed sbt assembly previously it will use that instead.
28+
export JAVA_OPTS="${JAVA_OPTS} -XX:MaxPermSize=256M -Xmx1024M -DloggerPath=conf/log4j.properties"
29+
ags="$@ generate -t modules/swagger-codegen/src/main/resources/slim -i modules/swagger-codegen/src/test/resources/2_0/petstore-security-test.yaml -l slim -o samples/server/petstore-security-test/slim"
30+
31+
java $JAVA_OPTS -jar $executable $ags

‎modules/swagger-codegen/src/main/java/io/swagger/codegen/CodegenConfig.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,12 @@ public interface CodegenConfig {
5757

5858
String escapeText(String text);
5959

60+
String escapeUnsafeCharacters(String input);
61+
6062
String escapeReservedWord(String name);
6163

64+
String escapeQuotationMark(String input);
65+
6266
String getTypeDeclaration(Property p);
6367

6468
String getTypeDeclaration(String name);

‎modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultCodegen.java‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -330,16 +330,43 @@ public void processSwagger(Swagger swagger) {
330330
// override with any special text escaping logic
331331
@SuppressWarnings("static-method")
332332
public String escapeText(String input) {
333-
if (input != null) {
334-
// remove \t, \n, \r
335-
// repalce \ with \\
336-
// repalce " with \"
337-
// outter unescape to retain the original multi-byte characters
338-
return StringEscapeUtils.unescapeJava(StringEscapeUtils.escapeJava(input).replace("\\/", "/")).replaceAll("[\\t\\n\\r]"," ").replace("\\", "\\\\").replace("\"", "\\\"");
333+
if (input == null) {
334+
return input;
339335
}
336+
337+
// remove \t, \n, \r
338+
// replace \ with \\
339+
// replace " with \"
340+
// outter unescape to retain the original multi-byte characters
341+
// finally escalate characters avoiding code injection
342+
return escapeUnsafeCharacters(StringEscapeUtils.unescapeJava(StringEscapeUtils.escapeJava(input).replace("\\/", "/")).replaceAll("[\\t\\n\\r]"," ").replace("\\", "\\\\").replace("\"", "\\\""));
343+
}
344+
345+
/**
346+
* override with any special text escaping logic to handle unsafe
347+
* characters so as to avoid code injection
348+
* @param input String to be cleaned up
349+
* @return string with unsafe characters removed or escaped
350+
*/
351+
public String escapeUnsafeCharacters(String input) {
352+
LOGGER.warn("escapeUnsafeCharacters should be overriden in the code generator with proper logic to escape unsafe characters");
353+
// doing nothing by default and code generator should implement
354+
// the logic to prevent code injection
355+
// later we'll make this method abstract to make sure
356+
// code generator implements this method
340357
return input;
341358
}
342359

360+
/**
361+
* Escape single and/or double quote to avoid code injection
362+
* @param input String to be cleaned up
363+
* @return string with quotation mark removed or escaped
364+
*/
365+
public String escapeQuotationMark(String input) {
366+
LOGGER.warn("escapeQuotationMark should be overriden in the code generator with proper logic to escape single/double quote");
367+
return input.replace("\"", "\\\"");
368+
}
369+
343370
public Set<String> defaultIncludes() {
344371
return defaultIncludes;
345372
}
@@ -1747,7 +1774,8 @@ public CodegenOperation fromOperation(String path, String httpMethod, Operation
17471774
int count = 0;
17481775
for (String key : consumes) {
17491776
Map<String, String> mediaType = new HashMap<String, String>();
1750-
mediaType.put("mediaType", key);
1777+
// escape quotation to avoid code injection
1778+
mediaType.put("mediaType", escapeQuotationMark(key));
17511779
count += 1;
17521780
if (count < consumes.size()) {
17531781
mediaType.put("hasMore", "true");
@@ -1780,7 +1808,8 @@ public CodegenOperation fromOperation(String path, String httpMethod, Operation
17801808
int count = 0;
17811809
for (String key : produces) {
17821810
Map<String, String> mediaType = new HashMap<String, String>();
1783-
mediaType.put("mediaType", key);
1811+
// escape quotation to avoid code injection
1812+
mediaType.put("mediaType", escapeQuotationMark(key));
17841813
count += 1;
17851814
if (count < produces.size()) {
17861815
mediaType.put("hasMore", "true");

‎modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultGenerator.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -144,36 +144,36 @@ public List<File> generate() {
144144
if (swagger.getInfo() != null) {
145145
Info info = swagger.getInfo();
146146
if (info.getTitle() != null) {
147-
config.additionalProperties().put("appName", info.getTitle());
147+
config.additionalProperties().put("appName", config.escapeText(info.getTitle()));
148148
}
149149
if (info.getVersion() != null) {
150-
config.additionalProperties().put("appVersion", info.getVersion());
150+
config.additionalProperties().put("appVersion", config.escapeText(info.getVersion()));
151151
}
152152
if (info.getDescription() != null) {
153153
config.additionalProperties().put("appDescription",
154154
config.escapeText(info.getDescription()));
155155
}
156156
if (info.getContact() != null) {
157157
Contact contact = info.getContact();
158-
config.additionalProperties().put("infoUrl", contact.getUrl());
158+
config.additionalProperties().put("infoUrl", config.escapeText(contact.getUrl()));
159159
if (contact.getEmail() != null) {
160-
config.additionalProperties().put("infoEmail", contact.getEmail());
160+
config.additionalProperties().put("infoEmail", config.escapeText(contact.getEmail()));
161161
}
162162
}
163163
if (info.getLicense() != null) {
164164
License license = info.getLicense();
165165
if (license.getName() != null) {
166-
config.additionalProperties().put("licenseInfo", license.getName());
166+
config.additionalProperties().put("licenseInfo", config.escapeText(license.getName()));
167167
}
168168
if (license.getUrl() != null) {
169-
config.additionalProperties().put("licenseUrl", license.getUrl());
169+
config.additionalProperties().put("licenseUrl", config.escapeText(license.getUrl()));
170170
}
171171
}
172172
if (info.getVersion() != null) {
173-
config.additionalProperties().put("version", info.getVersion());
173+
config.additionalProperties().put("version", config.escapeText(info.getVersion()));
174174
}
175175
if (info.getTermsOfService() != null) {
176-
config.additionalProperties().put("termsOfService", info.getTermsOfService());
176+
config.additionalProperties().put("termsOfService", config.escapeText(info.getTermsOfService()));
177177
}
178178
}
179179

@@ -184,7 +184,7 @@ public List<File> generate() {
184184
StringBuilder hostBuilder = new StringBuilder();
185185
String scheme;
186186
if (swagger.getSchemes() != null && swagger.getSchemes().size() > 0) {
187-
scheme = swagger.getSchemes().get(0).toValue();
187+
scheme = config.escapeText(swagger.getSchemes().get(0).toValue());
188188
} else {
189189
scheme = "https";
190190
}

‎modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/LumenServerCodegen.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,4 +215,16 @@ public String getSwaggerType(Property p) {
215215
type = swaggerType;
216216
return toModelName(type);
217217
}
218+
219+
@Override
220+
public String escapeQuotationMark(String input) {
221+
// remove ' to avoid code injection
222+
return input.replace("'", "");
223+
}
224+
225+
@Override
226+
public String escapeUnsafeCharacters(String input) {
227+
return input.replace("*/", "");
228+
}
229+
218230
}

‎modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/PhpClientCodegen.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,4 +662,16 @@ public Map<String, Object> postProcessOperations(Map<String, Object> objs) {
662662
}
663663
return objs;
664664
}
665+
666+
@Override
667+
public String escapeQuotationMark(String input) {
668+
// remove ' to avoid code injection
669+
return input.replace("'", "");
670+
}
671+
672+
@Override
673+
public String escapeUnsafeCharacters(String input) {
674+
return input.replace("*/", "");
675+
}
676+
665677
}

0 commit comments

Comments
 (0)