Skip to content

Commit

Permalink
attachment: Use SHA-256 instead of SHA-1
Browse files Browse the repository at this point in the history
Overwriting an existing attachment file maliciously is possible since
Yona uses SHA-1 algorithm, which is known to be shattered[1], to digest
the contents to generate the names of the attachment files.

This fix ensures backward compatibility. Only new attachments have
filenames generated by SHA-256 algorithm. Fortunately, the length of
'name' column of 'attachment' table is 255 which is enough to store
SHA-256 which requires 64.

[1]: https://shattered.it/
  • Loading branch information
eungjun-yi authored and doortts committed Mar 22, 2017
1 parent fb97b51 commit 21651f7
Showing 1 changed file with 7 additions and 7 deletions.
14 changes: 7 additions & 7 deletions app/models/Attachment.java
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public void moveTo(Resource to) {
* PlayFramework to the Upload Directory managed by Yobi.
*
* @param file
* @return SHA1 hash of the file
* @return SHA-256 hash of the file
* @throws NoSuchAlgorithmException
* @throws IOException
*/
Expand All @@ -212,7 +212,7 @@ private static File moveFileIntoUploadDirectory(File file)
// Compute sha1 checksum.
InputStream is = new FileInputStream(file);
byte buf[] = new byte[10240];
MessageDigest algorithm = MessageDigest.getInstance("SHA1");
MessageDigest algorithm = MessageDigest.getInstance("SHA-256");
for (int readSize = 0; readSize >= 0; readSize = is.read(buf)) {
algorithm.update(buf, 0, readSize);
}
Expand Down Expand Up @@ -240,7 +240,7 @@ private static File moveFileIntoUploadDirectory(File file, String hash)
* Attaches an uploaded file to the given container with the given name.
*
* Moves an uploaded file to the Upload Directory and rename the file to
* its SHA1 hash. And it stores the metadata of the file in this entity.
* its SHA-256 hash. And it stores the metadata of the file in this entity.
*
* If there is an entity that has the same values with this entity already,
* it means the container has the same attachment. If that is the case,
Expand Down Expand Up @@ -501,7 +501,7 @@ public boolean store(InputStream inputStream, @Nullable String fileName,
byte buf[] = new byte[10240];

// Compute hash and store the stream as a temp file
MessageDigest algorithm = MessageDigest.getInstance("SHA1");
MessageDigest algorithm = MessageDigest.getInstance("SHA-256");
String tempFileHash;
File tmpFile = File.createTempFile("yobi", null);
FileOutputStream fos = new FileOutputStream(tmpFile);
Expand Down Expand Up @@ -546,9 +546,9 @@ public static Page<Attachment> findByUser(User user, int pageSize, int pageNo, S
*/
private boolean save(File file, String fileName, Resource container) throws
IOException {
// Store the file as its SHA1 hash in filesystem, and record its
// metadata - containerType, containerId, createdDate, name, size, hash and
// mimeType - in Database.
// Store the file as its SHA-256 hash in filesystem, and record its
// metadata - containerType, containerId, createdDate, name, size, hash
// and mimeType - in Database.
this.containerType = container.getType();
this.containerId = container.getId();
this.createdDate = JodaDateUtil.now();
Expand Down

0 comments on commit 21651f7

Please sign in to comment.