From 984683ab4b44920c8be6af5779c246966cb4bcb4 Mon Sep 17 00:00:00 2001
From: David North <dnorth@apache.org>
Date: Mon, 1 Aug 2016 12:51:24 +0000
Subject: [PATCH] Fix zero-padding and handling of empty passwords (meaning
 protection on, but no password to remove it) for XSSF workbook protection.

https://bz.apache.org/bugzilla/show_bug.cgi?id=59920

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1754744 13f79535-47bb-0310-9956-ffa450edef68
---
 .../poi/poifs/crypt/CryptoFunctions.java      | 26 ++++++++++---------
 .../usermodel/helpers/XSSFPaswordHelper.java  |  4 +--
 .../poi/xssf/usermodel/TestXSSFSheet.java     | 25 ++++++++++++++++++
 3 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/src/java/org/apache/poi/poifs/crypt/CryptoFunctions.java b/src/java/org/apache/poi/poifs/crypt/CryptoFunctions.java
index f681f3ad1..69f8d6768 100644
--- a/src/java/org/apache/poi/poifs/crypt/CryptoFunctions.java
+++ b/src/java/org/apache/poi/poifs/crypt/CryptoFunctions.java
@@ -374,20 +374,22 @@ public class CryptoFunctions {
         // SET Verifier TO 0x0000
         short verifier = 0;
 
-        // FOR EACH PasswordByte IN PasswordArray IN REVERSE ORDER
-        for (int i = arrByteChars.length-1; i >= 0; i--) {
-            // SET Verifier TO Intermediate3 BITWISE XOR PasswordByte
+        if (!"".equals(password)) {
+            // FOR EACH PasswordByte IN PasswordArray IN REVERSE ORDER
+            for (int i = arrByteChars.length-1; i >= 0; i--) {
+                // SET Verifier TO Intermediate3 BITWISE XOR PasswordByte
+                verifier = rotateLeftBase15Bit(verifier);
+                verifier ^= arrByteChars[i];
+            }
+    
+            // as we haven't prepended the password length into the input array
+            // we need to do it now separately ...
             verifier = rotateLeftBase15Bit(verifier);
-            verifier ^= arrByteChars[i];
+            verifier ^= arrByteChars.length;
+            
+            // RETURN Verifier BITWISE XOR 0xCE4B
+            verifier ^= 0xCE4B; // (0x8000 | ('N' << 8) | 'K')
         }
-
-        // as we haven't prepended the password length into the input array
-        // we need to do it now separately ...
-        verifier = rotateLeftBase15Bit(verifier);
-        verifier ^= arrByteChars.length;
-        
-        // RETURN Verifier BITWISE XOR 0xCE4B
-        verifier ^= 0xCE4B; // (0x8000 | ('N' << 8) | 'K')
         
         return verifier & 0xFFFF;
     }
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFPaswordHelper.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFPaswordHelper.java
index c91049948..fd6e2dbd0 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFPaswordHelper.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFPaswordHelper.java
@@ -55,8 +55,8 @@ public class XSSFPaswordHelper {
         cur.toFirstContentToken();
         if (hashAlgo == null) {
             int hash = CryptoFunctions.createXorVerifier1(password);
-            cur.insertAttributeWithValue(getAttrName(prefix, "password"), 
-                                         Integer.toHexString(hash).toUpperCase(Locale.ROOT));
+            cur.insertAttributeWithValue(getAttrName(prefix, "password"),
+                                         String.format(Locale.ROOT, "%04X", hash).toUpperCase(Locale.ROOT));
         } else {
             SecureRandom random = new SecureRandom(); 
             byte salt[] = random.generateSeed(16);
diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java
index a9b0d1f0b..689e999bc 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFSheet.java
@@ -80,6 +80,7 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTWorksheet;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTXf;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCalcMode;
 import org.openxmlformats.schemas.spreadsheetml.x2006.main.STPane;
+import org.openxmlformats.schemas.spreadsheetml.x2006.main.STUnsignedShortHex;
 
 
 public final class TestXSSFSheet extends BaseTestXSheet {
@@ -1099,6 +1100,30 @@ public final class TestXSSFSheet extends BaseTestXSheet {
         wb.close();
     }
 
+    @Test
+    public void protectSheet_emptyPassword() throws IOException {
+        XSSFWorkbook wb = new XSSFWorkbook();
+        XSSFSheet sheet = wb.createSheet();
+        CTSheetProtection pr = sheet.getCTWorksheet().getSheetProtection();
+        assertNull("CTSheetProtection should be null by default", pr);
+        String password = "";
+        sheet.protectSheet(password);
+        pr = sheet.getCTWorksheet().getSheetProtection();
+        assertNotNull("CTSheetProtection should be not null", pr);
+        assertTrue("sheet protection should be on", pr.isSetSheet());
+        assertTrue("object protection should be on", pr.isSetObjects());
+        assertTrue("scenario protection should be on", pr.isSetScenarios());
+        int hashVal = CryptoFunctions.createXorVerifier1(password);
+        STUnsignedShortHex xpassword = pr.xgetPassword();
+        int actualVal = Integer.parseInt(xpassword.getStringValue(),16);
+        assertEquals("well known value for top secret hash should match", hashVal, actualVal);
+
+        sheet.protectSheet(null);
+        assertNull("protectSheet(null) should unset CTSheetProtection", sheet.getCTWorksheet().getSheetProtection());
+
+        wb.close();
+    }
+
     @Test
     public void protectSheet_lowlevel_2013() throws IOException {
         String password = "test";